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@xxxxxxxxxx> > --- > > 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list