On 04/30/14 06:22, Eric Blake wrote:
Commit f22b7899 stumbled across a difference between 32-bit and
64-bit platforms; on 64-bit, parsing "-1" as a long produces
0xffffffffffffffff, which does not fit in unsigned int; but on
32-bit, it parses as 0xffffffff, which DOES fit. Another patch
will tweak virStrToLong_ui to behave the same across platforms,
but regardless of which of the two choices it makes, the chain
lookup code wants to reject negative numbers rather than treating
it as large integers.
* src/util/virstoragefile.c (virStorageFileParseChainIndex):
Reject negative sign.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
Therefore, although this qualifies as a build-breaker fix on
32-bit, I haven't pushed it yet: I'm still working on my patch
to virstring.c for uniform behavior; and that turned up the
fact that virstringtest.c doesn't have coverage of integer
parsing, so of course, I have to expand that test too...
src/util/virstoragefile.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index dcce1ef..5d933a4 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1525,7 +1525,9 @@ virStorageFileParseChainIndex(const char *diskTarget,
if (virStringListLength(strings) != 2)
goto cleanup;
- if (virStrToLong_ui(strings[1], &suffix, 10, &idx) < 0 ||
+ /* Rule out spaces or negative sign */
+ if (!c_isdigit(*strings[1]) ||
I'd rather see a wrapper that rejects negative numbers when parsing into
a unsigned type rather than having it silently convert the number to a
negative one.
I know that there are places where this is desired, but they should be
fixed to explicitly use a differnent func.
+ virStrToLong_ui(strings[1], &suffix, 10, &idx) <
0 ||
STRNEQ(suffix, "]"))
goto cleanup;
I'm inclined to NACK this change.
Peter