qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/2] qemu-opts: introduce a function to compare


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 1/2] qemu-opts: introduce a function to compare option names
Date: Wed, 25 Jul 2012 19:31:32 +0100

On 25 July 2012 18:33, Anthony Liguori <address@hidden> wrote:
> Peter Maydell <address@hidden> writes:
>> This is not in line with the return value that the C library
>> strcmp() would return. C99 7.21.4 says "The sign of a nonzero
>> value returned by the comparison functions memcmp, strcmp,
>> and strncmp is determined by the sign of the difference between
>> the values of the first pair of characters (both interpreted
>> as unsigned char) that differ in the objects being compared."
>> This code will use whatever the signed/unsignedess of plain
>> 'char' is.
>>
>> (None of the callers use the sign of the return value so this
>> is something of a nitpick.)
>
> Sorry, how is this wrong?
>
> This returns:
>
> strcmp("a", "b") -> -1
> qemu_opt_name_cmp("a", "b") -> -1

 strcmp("\x90", "\x50") -> 64
 qemu_opt_name_cmp("\x90", "\x50") -> -1

(assuming a 'char is signed' architecture like x86, or use
"gcc -fsigned-char" if by some miracle you do your qemu
development on an ARM box :-))

>> If you made the for() loop into a do..while so that we
>> execute the loop body for the 'final NUL' case you could
>> avoid having this pair of checks, I think (and you can
>> make the loop termination case just '!lhs[i]' since if
>> we get past the 'mismatch' exits we've definitely got
>> to the end of both strings and can return true).
>
> I can poke around but not convinced it will result in better code.  I
> must admit that I prefer explicit handling of edge cases like this.

Typically these basic C library functions are arranged so
that the edge cases mostly fall out in the wash
(because the original implementation was done to be simple
to code rather than to provide particular semantics),
so if I see an implementation that does special case them
it makes me wonder if it's getting something wrong.

For instance the BSD naive strcmp is just four lines and
no special casing apart from "stop if we ran out of string":

    while (*s1 == *s2++)
        if (*s1++ == '\0')
            return (0);
    return (*(const unsigned char *)s1 - *(const unsigned char *)(s2 - 1));

-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]