MISRA C Rule 2.1 states: "A project shall not contain unreachable code."
Function `PrintErrMesg(const CHAR16*, EFI_STATUS)` isn't intended to return
control to its caller. At the end, it calls `blexit()`, which, in turn,
invokes the `__builtin_unreachable()` function, making subsequent return
statements in `read_file()` unreachable:
PrintErrMesg(name, ret);
/* not reached */
return false;
This also causes unreachability of the code meant to handle `read_file()`
errors, as seen in these examples:
In this block:
if ( read_file(dir_handle, file_name, &cfg, NULL) )
break;
*tail = 0;
}
if ( !tail )
blexit(L"No configuration file found.");
And here:
else if ( !read_file(dir_handle, cfg_file_name, &cfg, NULL) )
blexit(L"Configuration file not found.");
And here:
if ( !read_file(dir_handle, s2w(&name), &cfg, NULL) )
{
PrintStr(L"Chained configuration file '");
PrintStr(name.w);
efi_bs->FreePool(name.w);
blexit(L"'not found.");
}
The issue arises because when an error occurs inside `read_file()`, it calls
`PrintErrMesg()` and does not return to the caller.
To address this the following changes are applied:
1. Remove `PrintErrMesg(name, ret);` from the `read_file()` function.
2. Replaced it with `PrintErr(name);`, which prints the file name and returns
control to the caller.
3. Change the `read_file()` return type from `bool` to `EFI_STATUS`, allowing
file operation result codes to be returned to the caller.
4. Properly handle error codes returned from the `read_file()` function in the
relevant areas of the code.
5. Replace `blexit()` calls with informative error codes using `PrintErrMesg()`
where appropriate.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopch...@epam.com>
---
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1980590118
---
xen/common/efi/boot.c | 57 ++++++++++++++++++++++++++++++-------------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 50ff1d1bd2..ddbafb2f9c 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -132,7 +132,7 @@ struct file {
};
};
-static bool read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
+static EFI_STATUS read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
struct file *file, const char *options);
static bool read_section(const EFI_LOADED_IMAGE *image, const CHAR16 *name,
struct file *file, const char *options);
@@ -782,7 +782,7 @@ static void __init handle_file_info(const CHAR16 *name,
efi_arch_handle_module(file, name, options);
}
-static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
+static EFI_STATUS __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
struct file *file, const char *options)
{
EFI_FILE_HANDLE FileHandle = NULL;
@@ -791,7 +791,7 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle,
CHAR16 *name,
const CHAR16 *what = NULL;
if ( !name )
- PrintErrMesg(L"No filename", EFI_OUT_OF_RESOURCES);
+ return EFI_INVALID_PARAMETER;
what = L"Open";
if ( dir_handle )
@@ -842,7 +842,7 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle,
CHAR16 *name,
efi_arch_flush_dcache_area(file->ptr, file->size);
- return true;
+ return ret;
fail:
if ( FileHandle )
@@ -850,10 +850,9 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle,
CHAR16 *name,
PrintErr(what);
PrintErr(L" failed for ");
- PrintErrMesg(name, ret);
+ PrintErr(name);
- /* not reached */
- return false;
+ return ret;
}
static bool __init read_section(const EFI_LOADED_IMAGE *image,
@@ -1433,18 +1432,20 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
while ( (tail = point_tail(file_name)) != NULL )
{
wstrcpy(tail, L".cfg");
- if ( read_file(dir_handle, file_name, &cfg, NULL) )
+ if ( (status = read_file(dir_handle, file_name,
+ &cfg, NULL)) == EFI_SUCCESS )
break;
*tail = 0;
}
if ( !tail )
- blexit(L"No configuration file found.");
+ PrintErrMesg(L"Configuration file failure.", status);
PrintStr(L"Using configuration file '");
PrintStr(file_name);
PrintStr(L"'\r\n");
}
- else if ( !read_file(dir_handle, cfg_file_name, &cfg, NULL) )
- blexit(L"Configuration file not found.");
+ else if ( (status = read_file(dir_handle, cfg_file_name,
+ &cfg, NULL)) != EFI_SUCCESS )
+ PrintErrMesg(L"Configuration file failure.", status);
pre_parse(&cfg);
if ( section.w )
@@ -1465,12 +1466,13 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
cfg.need_to_free = false;
}
- if ( !read_file(dir_handle, s2w(&name), &cfg, NULL) )
+ if ( (status = read_file(dir_handle, s2w(&name),
+ &cfg, NULL)) != EFI_SUCCESS )
{
- PrintStr(L"Chained configuration file '");
+ PrintStr(L"Configuration file '");
PrintStr(name.w);
efi_bs->FreePool(name.w);
- blexit(L"'not found.");
+ PrintErrMesg(L"'failure.", status);
}
pre_parse(&cfg);
efi_bs->FreePool(name.w);
@@ -1483,7 +1485,14 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
if ( !read_section(loaded_image, L"kernel", &kernel, option_str) &&
name.s )
{
- read_file(dir_handle, s2w(&name), &kernel, option_str);
+ status = read_file(dir_handle, s2w(&name), &kernel, option_str);
+ if ( status != EFI_SUCCESS )
+ {
+ PrintStr(L"Configuration file '");
+ PrintStr(name.w);
+ efi_bs->FreePool(name.w);
+ PrintErrMesg(L"'failure.", status);
+ }
efi_bs->FreePool(name.w);
}
else
@@ -1497,7 +1506,14 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
name.s = get_value(&cfg, section.s, "ramdisk");
if ( name.s )
{
- read_file(dir_handle, s2w(&name), &ramdisk, NULL);
+ status = read_file(dir_handle, s2w(&name), &ramdisk, NULL);
+ if ( status != EFI_SUCCESS )
+ {
+ PrintStr(L"Configuration file '");
+ PrintStr(name.w);
+ efi_bs->FreePool(name.w);
+ PrintErrMesg(L"'failure.", status);
+ }
efi_bs->FreePool(name.w);
}
}
@@ -1507,7 +1523,14 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
name.s = get_value(&cfg, section.s, "xsm");
if ( name.s )
{
- read_file(dir_handle, s2w(&name), &xsm, NULL);
+ status = read_file(dir_handle, s2w(&name), &xsm, NULL);
+ if ( status != EFI_SUCCESS )
+ {
+ PrintStr(L"Configuration file '");
+ PrintStr(name.w);
+ efi_bs->FreePool(name.w);
+ PrintErrMesg(L"'failure.", status);
+ }
efi_bs->FreePool(name.w);
}
}
--
2.43.0
