Re: [Qemu-devel] [PATCH v6 9/9] tests: Add uuid tests

[Eric Blake]

On 08/17/2016 02:28 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  tests/Makefile.include |   2 +
>  tests/test-uuid.c      | 173 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 175 insertions(+)
>  create mode 100644 tests/test-uuid.c
> 

> +    }, { /* Double '-' */
> +        "0cc6c752--3961-4028-a286-c05cc616d396",
> +    }, { /* Too long */
> +        "0000000000000000000000000000000000000000000000",
> +    }, { /* Invalid char in the beginning */
> +        ")cc6c752--3961-4028-a286-c05cc616d396",

Umm, why the double dash in this and the remaining tests?  They are
failing for the wrong reason.

> +    }, { /* Invalid char in the beginning, in extra */
> +        ")0cc6c752--3961-4028-a286-c05cc616d396",
> +    }, { /* Invalid char in the middle */
> +        "0cc6c752--39*1-4028-a286-c05cc616d396",
> +    }, { /* Invalid char in the middle, in extra */
> +        "0cc6c752--39*61-4028-a286-c05cc616d396",
> +    }, { /* Invalid char in the end */
> +        "0cc6c752--3961-4028-a286-c05cc616d39&",
> +    }, { /* Invalid char in the end, in extra */
> +        "0cc6c752--3961-4028-a286-c05cc616d396&",

And, I was able to quickly prove that you are missing a couple of tests:

    }, { /* Leading space and short end */
        " 0cc6c752-3961-4028-a286-c05cc616d39",
    },

Whoops - that test fails.
GTESTER tests/test-uuid
**
ERROR:tests/test-uuid.c:126:test_uuid_parse: assertion failed (!r ==
is_valid): (1 == 0)

Yay for sscanf() not doing what we want.  :(  We've had a latent bug for
as long as we've been calling sscanf(UUID_FMT).

Why does it fail, you ask?  Because sscanf() skips leading spaces, and
doesn't start counting the %02hhx parse until after the leading space;
then the final %02hhx parse happily stops parsing with just 1 character.
 The string is the right length, but the wrong format.

Also:

    }, { /* Short end and trailing space */
        "0cc6c752-3961-4028-a286-c05cc616d39 ",

You may want to add a followup patch that tightens parsing; perhaps
using strspn() to double check that not only is the input string the
right length, but also that it only has [a-fA-F0-9-], so that any
whitespace that sneaks past sscanf() is detected.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature