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

[Roy Franz <roy.franz@xxxxxxxxxx>]

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:
>> +    /*
>> +     * Make a copy, as conversion is destructive, and caller still wants
>> +     * wide string available after this call returns.
>> +     */
>> +    if ( efi_bs->AllocatePool(EfiLoaderData, (wstrlen(name) + 1) * 
>> sizeof(*name),
>> +                              &ptr) != EFI_SUCCESS )
>> +        blexit(L"ERROR Unable to allocate string buffer");
>
> Kind of disappointing: You said you'd drop these ERROR prefixes,
> but this is the second one I come across. I'm ditching them in
> preparation for committing.
>
>> +
>> +    local_name.w = ptr;
>> +    wstrcpy(local_name.w, name);
>> +    w2s(&local_name);
>> +
>> +    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...
>

I apologize - I missed adding the above to items to my checklist, and
was likely too focused
on getting an updated set out yesterday so these items got missed.

>> @@ -861,16 +882,16 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
>> *SystemTable)
>>      name.s = get_value(&cfg, section.s, "ramdisk");
>>      if ( name.s )
>>      {
>> -        split_value(name.s);
>> -        read_file(dir_handle, s2w(&name), &ramdisk);
>> +        option_str = split_string(name.s);
>> +        read_file(dir_handle, s2w(&name), &ramdisk, option_str);
>
> As said before, this should be NULL as not having and never going to
> have a consumer. Whether you keep the XSM one below is - as also
> said before - up to you.
>
> Jan

I'll drop the options for ramdisk and xsm.  I addresses the options on
the ucode module,
which is what you had previously commented on.

I will rebase on staging ( or master if they get pushed there today)
to get the patches you have already committed.

Roy

>
>>          efi_bs->FreePool(name.w);
>>      }
>>
>>      name.s = get_value(&cfg, section.s, "xsm");
>>      if ( name.s )
>>      {
>> -        split_value(name.s);
>> -        read_file(dir_handle, s2w(&name), &xsm);
>> +        option_str = split_string(name.s);
>> +        read_file(dir_handle, s2w(&name), &xsm, option_str);
>>          efi_bs->FreePool(name.w);
>>      }
>>
>
>

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