On 04/30/2014 10:27 PM, Eric Blake wrote: > v1 was here, but it got nacked for being ugly: > https://www.redhat.com/archives/libvir-list/2014-April/msg01132.html > > so in this version, I improved virstring first, then used the > new clean function. > > This is a build-breaker fix, so it deserves to be in 1.2.4; > but it is large enough that it needs a review to go in. > > Eric Blake (3): > util: fix uint parsing on 64-bit platforms > util: new stricter unsigned int parsing > storage: reject negative indices > > src/libvirt_private.syms | 3 + > src/util/virstoragefile.c | 2 +- > src/util/virstring.c | 98 +++++++++++++++++++++-- > src/util/virstring.h | 14 +++- > tests/virstringtest.c | 200 +++++++++++++++++++++++++++++++++++++++++++++- > 5 files changed, 307 insertions(+), 10 deletions(-) > In short ACK... Since I believe this resolves the issue seen. Oh the games we have to play because 32 (and 16) bit computing still exists... I've looked at the changes... Ran them thru coverity... Things seem OK to me. Although we may have some work to do to check users of the existing API's... Having to "read minds" about what someone "really" meant when they used -1 in a CLI/API can be dangerous. Was using -1 meant to produce an error or was it meant to be a shortcut for a largest value or was it a result of using a 64 bit value in order to manipulate a 32 bit number? If you know the underlying code doesn't accept (or want) negative numbers, then -1 would be bad. My favorite is of course finding those places where someone uses ~0ULL (or some variant). Having to ensure/describe what each datum is for each API/CLI reference could be a time consuming excursion through a lot of code in order to determine and validate what "should" and/or "could" be passed. The other side of the coin is - what happens on display if say someone indicates -1 on input (for whatever reason) - does the display side then display the maximum whatever value or does it (or could/should it) display the -1? One example that comes to mind (because of a recent changes (c4206d7 and dff3ad0) that affected virt-test/tp-libvirt results) is from migrate-setspeed where the maximum value allowed is INT64_MAX (although it's not documented as that in the man page)... Although the recent changes don't allow passing -1 thru the virsh command - when they did - the output on a getspeed would be the 1844*1615 value (UINT64_MAX). The bz associated indicated use of -1 by the tester in order to force use of the maximum value, which is where it caught my attention as being somewhat related to this particular change... John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list