Re: [Xen-devel] [PATCH] xen/cmdline: Fix buggy strncmp(s, LITERAL, ss - s) construct

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.19 at 16:55, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 04/01/2019 15:32, Jan Beulich wrote:
>>>>> On 31.12.18 at 18:35, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> +    {
>>> +        int res = (*frag - *name);
>> With the result of this being implementation defined (due to plain
>> char's implementation defined - often command line controlled
>> with an implementation defined default - signedness) I wonder if
>> this function can really usefully return "int" rather than "bool".
> 
> My CPUID command line parsing needs this to work properly as int, for
> bisecting across a sorted list.
> 
> I'll add an explicit cast to signed char.  I'll also fix out local libc
> functions, which are similarly buggy.

Why "signed char" when the standard specifically mandates "unsigned
char"?

>>> +        if ( res || *name == '\0' )
>>> +        {
>>> +            /*
>>> +             * NUL in 'name' matching a comma or colon in 'frag' implies
>>> +             * success.
>>> +             */
>>> +            if ( *name == '\0' && (*frag == ',' || *frag == ':') )
>> There's only a single (unrelated) use of ; as a separator right
>> now (afaics), but adding it here would seem quite desirable to
>> me.
> 
> Where is ; used, out of interest?  I'm happy to add it, but I didn't
> spot it on my audit.

As said, it's in unrelated (but still command line parsing) code.
See xen/drivers/passthrough/vtd/dmar.c:parse_rmrr_param().

>> Also, speaking of (the lack of) tokenization of the command line
>> in the caller, wouldn't it make sense to accept white space as
>> separators here too?
> 
> I'm not sure if that is wise or not.  I can't think of a situation where
> you would want that behaviour, rather than finding yourself with a
> parsing bug and having to fix a bug elsewhere.

Well, I wasn't sure either (hence me having phrased it as a
question), so let's leave it out (at least for now)

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel