Re: [Xen-devel] [PATCH v3 33/52] xen/drivers/passthrough/iommu.c: let custom parameter parsing routines return errno

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.08.17 at 11:27, <jgross@xxxxxxxx> wrote:
> On 22/08/17 12:04, Jan Beulich wrote:
>>>>> On 16.08.17 at 14:52, <jgross@xxxxxxxx> wrote:
>>> @@ -89,44 +89,50 @@ static void __init parse_iommu_param(char *s)
>>>              s += 3;
>>>  
>>>          ss = strchr(s, ',');
>>> -        if ( ss )
>>> -            *ss = '\0';
>>> -
>>> -        if ( !parse_bool(s) )
>>> -            iommu_enable = 0;
>>> -        else if ( !strcmp(s, "force") || !strcmp(s, "required") )
>>> +        if ( !ss )
>>> +            ss = strchr(s, '\0');
>>> +
>>> +        b = parse_bool(s);
>> 
>> I don't think this will work as intended for "iommu=yes,...". Did I
>> perhaps overlook the same issue in some of the earlier patches?
> 
> I'm just of the opposite opinion: the former implementation didn't work
> as intended in that case:
> 
> Let a hypervisor be built with a pre-defined command line "iommu=no".
> A user supplied command line "iommu=yes" wouldn't change iommu_enable.
> 
> My patch changes that: setting iommu_enable to 0 or 1 in case
> parse_bool() succeeded is the correct thing to do.
> 
> Maybe I should mention this in the commit message, though.

That's an orthogonal aspect, which I agree with. My comment
was about parse_bool() expecting a nul-terminated "yes" (or
alike), though.

Jan


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