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

Re: [Xen-devel] [PATCH v3 39/52] xen: check parameter validity when parsing command line



>>> On 23.08.17 at 16:21, <jgross@xxxxxxxx> wrote:
> On 23/08/17 15:18, Jan Beulich wrote:
>>>>> On 23.08.17 at 14:42, <jgross@xxxxxxxx> wrote:
>>> On 23/08/17 11:38, Jan Beulich wrote:
>>>>>>> On 23.08.17 at 11:30, <jgross@xxxxxxxx> wrote:
>>>>> On 22/08/17 13:24, Jan Beulich wrote:
>>>>>>>>> On 16.08.17 at 14:52, <jgross@xxxxxxxx> wrote:
>>>>>>> @@ -176,7 +210,8 @@ int __init parse_bool(const char *s)
>>>>>>>           !strcmp("on", s) ||
>>>>>>>           !strcmp("true", s) ||
>>>>>>>           !strcmp("enable", s) ||
>>>>>>> -         !strcmp("1", s) )
>>>>>>> +         !strcmp("1", s) ||
>>>>>>> +         !*s )
>>>>>>>          return 1;
>>>>>>
>>>>>> Careful with this: Taking the "iommu=" example that I've commented
>>>>>> on in the other patch already, much depends on what you mean to
>>>>>> do about the problem there: "iommu=,..." should not end up
>>>>>> meaning "iommu=on,...".
>>>>>
>>>>> It won't. *s will be ',' in this case.
>>>>
>>>> Right, but as said - much depends on what you mean to do about
>>>> the problem in the earlier patch.
>>>
>>> So I just hit this. And looking more thoroughly into it: today it in
>>> fact has exactly this meaning. iommu_enable is "1" per default. So
>>> specifying "iommu=,..." won't change this and has the same semantics
>>> as "iommu=on,...".
>> 
>> But that's not the interesting case. Are you saying that
>> "iommu=off iommu=,..." enables the IOMMU today? It doesn't
>> look to me as if it would.
> 
> I guess this is a weird case after all: trying to be compatible to
> "iommu=off iommu=,..." while "iommu=off iommu=on,..." doesn't work
> correctly today is questionable.

Yeah, it shouldn't be "compatibility", but "sane behavior".

> OTOH the easiest way to avoid all discussions might be to drop the
> parse_bool() modification to return "true" in case of an empty string
> and handle this case by not calling parse_bool() at all from
> _cmdline_parse() if no argument was given.

Good idea.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.