Re: [Xen-devel] [PATCH for-4.5 V7 11/14] Add arch specific module handling to read_file()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.09.14 at 02:25, <roy.franz@xxxxxxxxxx> wrote:
> On Thu, Sep 25, 2014 at 11:52 AM, Roy Franz <roy.franz@xxxxxxxxxx> wrote:
>> On Thu, Sep 25, 2014 at 9:44 AM, Roy Franz <roy.franz@xxxxxxxxxx> wrote:
>>> On Thu, Sep 25, 2014 at 3:34 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>> On 25.09.14 at 03:42, <roy.franz@xxxxxxxxxx> wrote:
>>>>> +    place_string(&mb_modules[mbi.mods_count].string, options);
>>>>> +    place_string(&mb_modules[mbi.mods_count].string, "");
>>>>
>>>> Hmm, this one's still here, and I don't recall having seen an
>>>> explanation for it. I guess I need to cut off committing at this
>>>> patch then...
>>>>
>>
>> The original code put the entire line from the config file into the
>> mb_modules[i].string field,
>> which consists of the filename and module options, space separated.
>> This was done at the
>> same time the filename was extracted to pass to read_file().
>>
>> My refactored version of read_file() takes a filename and option
>> string as separate arguments,
>> and then after the file is loaded and address known, the module info is 
> updated.
>> The efi_arch_handle_module() gets the filename and options separately,
>> and the above place_string()
>> commands reconstruct the the single string of filename followed by
>> options that is placed
>> in the mb_modules[i].string field.
>>
>> I will add a comment to this effect, and also only add the options if
>> present, since for
>> several module types options will be NULL.
>>
> 
> Reviewing this in more detail, the line:
> 
> place_string(&mb_modules[mbi.mods_count].string, "");
> 
> is not needed and has been removed.  I have verified the same kernel 
> commandline
> in dom0 (/proc/cmdline) before and after the change.

Indeed, since

    if ( s && *s )

in place_string() made this a no-op.

Jan


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