[libvirt] [PATCH] conf: handle 'vda[-1]' uniformly

[Peter Krempa]

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 at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140430/92c82c40/attachment-0001.sig>