[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xen/cmdline: Fix buggy strncmp(s, LITERAL, ss - s) construct
>>> 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |