Re: [libvirt] [PATCH] virstoragetest: Don't run the test on 32 bit arches

On 04/29/2014 02:36 AM, Michal Privoznik wrote: > Currently, there's an issue with virStrToLong_* APIs that they turn > "-1" into UINT_MAX. While this is not acceptable, it works on 64 bit > architectures and doesn't work on 32 bit ones. I know that much > cleaner solution is required, but given that we are in the freeze we > may as well just skip the test on 32 bits. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > tests/virstoragetest.c | 54 ++++++++++++++++++++++++++++++++------------------ > 1 file changed, 35 insertions(+), 19 deletions(-) As discussed earlier, I'm proposing an alternative patch (series). Part one is here: https://www.redhat.com/archives/libvir-list/2014-April/msg01132.html and I tested that it lets the test pass on 32-bit builds again, with a MUCH smaller diffstat and no loss of test coverage. Part two is still under development (I'm in the middle of enhancing tests/virstringtest.c to actually cover things), but here's the diff I'm currently playing with: diff --git i/src/util/virstring.c w/src/util/virstring.c index 64c7259..c646669 100644 --- i/src/util/virstring.c +++ w/src/util/virstring.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2012-2013 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -221,6 +221,15 @@ virStrToLong_ui(char const *s, char **end_ptr, int base, unsigned int *result) errno = 0; val = strtoul(s, &p, base); /* exempt from syntax-check */ + + /* This one's tricky. We _want_ to allow "-1" as shorthand for

+ * UINT_MAX, but strtoul() treats "-1" as ULONG_MAX; casting from + * ulong back to uint changes the values only on platforms where + * long is a larger size. */ + if ((val & 0xffffffff00000000ULL) == 0xffffffff00000000ULL && + memchr(s, '-', p - s)) + val &= 0xffffffff;

+ err = (errno || (!end_ptr && *p) || p == s || (unsigned int) val != val); if (end_ptr) *end_ptr = p;