On Thu, Jul 24, 2014 at 12:09 AM, Jan Beulich JBeulich@suse.com wrote:
On 22.07.14 at 02:43, roy.franz@linaro.org wrote:
The read_file() function updated some multiboot specific data structures as it was loading a file. These changes make read_file() more generic, and create a load_file() wrapper for x86 that updates the multiboot data structures. read_file() no longer does special handling of the configuration file, as this was only needed to avoid adding it to the multiboot structures. read_file() and load_file() return error codes rather than directly exiting on error to facilicate sharing. Different architectures may require different max allocation addresses so take that as an argument.
Unless you expect an architecture to pass in different values on different invocations this clearly can be a #define rather than a function parameter.
I'll remove the argument - Ian's module freeing patch removes the need for this.
@@ -405,12 +399,21 @@ static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
if ( what ) {
PrintErr(what);PrintErr(L" failed for ");PrintErrMesgExit(name, ret);
PrintErrMesg(what, ret);PrintErr(L"Unable to load file");Is it intentional to make the message less useful by dripping the printing of the file name?
No, I have fixed this.
return 0;- }
- else
No need for an "else" after an unconditional "return" in the "if" branch.
@@ -470,6 +473,21 @@ static char *__init get_value(const struct file *cfg, const char *section, } return NULL; } +/* Only call with non-config files. */
Missing blank line before this comment.
+bool_t __init load_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
struct file *file)+{
- EFI_PHYSICAL_ADDRESS max = min(1UL << (32 + PAGE_SHIFT),
HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);- if ( read_file(dir_handle, name, file, max) )
Missing blank line between declaration and first statement.
Fixed this and above issues
@@ -896,16 +921,20 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) { microcode_set_module(mbi.mods_count); split_value(name.s);
read_file(dir_handle, s2w(&name), &ucode);
load_ok = load_file(dir_handle, s2w(&name), &ucode); efi_bs->FreePool(name.w);if ( !load_ok )blexit(L"Unable to load ucode image.");}
name.s = get_value(&cfg, section.s, "xsm"); if ( name.s ) { split_value(name.s);
read_file(dir_handle, s2w(&name), &xsm);
load_ok = load_file(dir_handle, s2w(&name), &xsm); efi_bs->FreePool(name.w);if ( !load_ok )blexit(L"Unable to load ucode image.");Apart from the same comment made on an earlier patch - no need for an extra message here when the called function already printed one - this is a copy'n'paste mistake: You mean XSM instead of ucode here.
Fixed, and I'll review redundant messages.
Jan