On 11.01.21 09:02, [ext] Christian Storm wrote:
> From: Christian Storm <[email protected]>
>
> Introduce ERROR(), WARNING(), and INFO() console message
> output macros to classify the messages and make their output
> more colorful so to increase readability for EBG's core output.
>
> Signed-off-by: Christian Storm <[email protected]>
> ---
Patch revision changelog here would be appreciated. I assume you only
removed also the prototype of Color().
Jan
> env/fatvars.c | 44 ++++++++++++++++++++++----------------------
> env/syspart.c | 14 +++++++-------
> include/utils.h | 20 +++++++++++++++++++-
> main.c | 14 ++++++--------
> utils.c | 49 ++++++++++++++++++++++++++++---------------------
> 5 files changed, 82 insertions(+), 59 deletions(-)
>
> diff --git a/env/fatvars.c b/env/fatvars.c
> index 99ac37e..e86743e 100644
> --- a/env/fatvars.c
> +++ b/env/fatvars.c
> @@ -28,20 +28,20 @@ BG_STATUS save_current_config(void)
>
> config_volumes = (UINTN *)mmalloc(sizeof(UINTN) * volume_count);
> if (!config_volumes) {
> - Print(L"Error, could not allocate memory for config partition "
> + ERROR(L"Could not allocate memory for config partition "
> L"mapping.\n");
> return result;
> }
>
> if (EFI_ERROR(enumerate_cfg_parts(config_volumes, &numHandles))) {
> - Print(L"Error, could not enumerate config partitions.\n");
> + ERROR(L"Could not enumerate config partitions.\n");
> goto scc_cleanup;
> }
>
> numHandles = filter_cfg_parts(config_volumes, numHandles);
>
> if (numHandles != ENV_NUM_CONFIG_PARTS) {
> - Print(L"Error, unexpected number of config partitions: found "
> + ERROR(L"Unexpected number of config partitions: found "
> L"%d, but expected %d.\n",
> numHandles, ENV_NUM_CONFIG_PARTS);
> /* In case of saving, this must be treated as error, to not
> @@ -54,7 +54,7 @@ BG_STATUS save_current_config(void)
> efistatus = open_cfg_file(v->root, &fh, EFI_FILE_MODE_WRITE |
> EFI_FILE_MODE_READ);
> if (EFI_ERROR(efistatus)) {
> - Print(L"Error, could not open environment file on system "
> + ERROR(L"Could not open environment file on system "
> L"partition %d: %r\n",
> current_partition, efistatus);
> goto scc_cleanup;
> @@ -68,13 +68,13 @@ BG_STATUS save_current_config(void)
> efistatus = uefi_call_wrapper(fh->Write, 3, fh, &writelen,
> (VOID *)&env[current_partition]);
> if (EFI_ERROR(efistatus)) {
> - Print(L"Error writing environment to file: %r\n", efistatus);
> + ERROR(L"Cannot write environment to file: %r\n", efistatus);
> (VOID) close_cfg_file(v->root, fh);
> goto scc_cleanup;
> }
>
> if (EFI_ERROR(close_cfg_file(v->root, fh))) {
> - Print(L"Error, could not close environment config file.\n");
> + ERROR(L"Could not close environment config file.\n");
> goto scc_cleanup;
> }
>
> @@ -94,25 +94,25 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
>
> config_volumes = (UINTN *)mmalloc(sizeof(UINTN) * volume_count);
> if (!config_volumes) {
> - Print(L"Error, could not allocate memory for config partition "
> + ERROR(L"Could not allocate memory for config partition "
> L"mapping.\n");
> return result;
> }
>
> if (EFI_ERROR(enumerate_cfg_parts(config_volumes, &numHandles))) {
> - Print(L"Error, could not enumerate config partitions.\n");
> + ERROR(L"Could not enumerate config partitions.\n");
> goto lc_cleanup;
> }
>
> numHandles = filter_cfg_parts(config_volumes, numHandles);
>
> if (numHandles > ENV_NUM_CONFIG_PARTS) {
> - Print(L"Error, too many config partitions found. Aborting.\n");
> + ERROR(L"Too many config partitions found. Aborting.\n");
> goto lc_cleanup;
> }
>
> if (numHandles < ENV_NUM_CONFIG_PARTS) {
> - Print(L"Warning, too few config partitions: found: "
> + WARNING(L"Too few config partitions: found: "
> L"%d, but expected %d.\n",
> numHandles, ENV_NUM_CONFIG_PARTS);
> /* Don't treat this as error because we may still be able to
> @@ -126,7 +126,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
> VOLUME_DESC *v = &volumes[config_volumes[i]];
> if (EFI_ERROR(open_cfg_file(v->root, &fh,
> EFI_FILE_MODE_READ))) {
> - Print(L"Warning, could not open environment file on "
> + WARNING(L"Could not open environment file on "
> L"config partition %d\n",
> i);
> result = BG_CONFIG_PARTIALLY_CORRUPTED;
> @@ -134,12 +134,12 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
> }
> UINTN readlen = sizeof(BG_ENVDATA);
> if (EFI_ERROR(read_cfg_file(fh, &readlen, (VOID *)&env[i]))) {
> - Print(L"Error reading environment from config "
> + ERROR(L"Cannot read environment from config "
> L"partition %d.\n",
> i);
> env_invalid[i] = 1;
> if (EFI_ERROR(close_cfg_file(v->root, fh))) {
> - Print(L"Error, could not close environment "
> + ERROR(L"Could not close environment "
> L"config file.\n");
> }
> result = BG_CONFIG_PARTIALLY_CORRUPTED;
> @@ -149,11 +149,11 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
> uint32_t crc32 = calc_crc32(&env[i], sizeof(BG_ENVDATA) -
> sizeof(env[i].crc32));
> if (crc32 != env[i].crc32) {
> - Print(L"CRC32 error in environment data on config "
> + ERROR(L"CRC32 error in environment data on config "
> L"partition %d.\n",
> i);
> - Print(L"calculated: %lx\n", crc32);
> - Print(L"stored: %lx\n", env[i].crc32);
> + INFO(L"calculated: %lx\n", crc32);
> + INFO(L"stored: %lx\n", env[i].crc32);
> /* Don't treat this as fatal error because we may still
> * have
> * valid environments */
> @@ -162,7 +162,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
> }
>
> if (EFI_ERROR(close_cfg_file(v->root, fh))) {
> - Print(L"Error, could not close environment config "
> + ERROR(L"Could not close environment config "
> L"file.\n");
> /* Don't abort, so we may still be able to boot a
> * config */
> @@ -220,12 +220,12 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
> StrDuplicate(env[current_partition].kernelparams);
> bglp->timeout = env[current_partition].watchdog_timeout_sec;
>
> - Print(L"Config Revision: %d:\n", latest_rev);
> - Print(L" ustate: %d\n",
> + INFO(L"Config Revision: %d:\n", latest_rev);
> + INFO(L" ustate: %d\n",
> env[current_partition].ustate);
> - Print(L" kernel: %s\n", bglp->payload_path);
> - Print(L" args: %s\n", bglp->payload_options);
> - Print(L" timeout: %d seconds\n", bglp->timeout);
> + INFO(L" kernel: %s\n", bglp->payload_path);
> + INFO(L" args: %s\n", bglp->payload_options);
> + INFO(L" timeout: %d seconds\n", bglp->timeout);
>
> result = BG_SUCCESS;
> lc_cleanup:
> diff --git a/env/syspart.c b/env/syspart.c
> index 5e248e5..44a18d3 100644
> --- a/env/syspart.c
> +++ b/env/syspart.c
> @@ -24,7 +24,7 @@ EFI_STATUS enumerate_cfg_parts(UINTN *config_volumes, UINTN
> *numHandles)
> UINTN rootCount = 0;
>
> if (!config_volumes || !numHandles) {
> - Print(L"Invalid parameter in system partition enumeration.\n");
> + ERROR(L"Invalid parameter in system partition enumeration.\n");
> return EFI_INVALID_PARAMETER;
> }
> for (UINTN index = 0; index < volume_count && rootCount < *numHandles;
> @@ -36,19 +36,19 @@ EFI_STATUS enumerate_cfg_parts(UINTN *config_volumes,
> UINTN *numHandles)
> status = open_cfg_file(volumes[index].root, &fh,
> EFI_FILE_MODE_READ);
> if (status == EFI_SUCCESS) {
> - Print(L"Config file found on volume %d.\n", index);
> + INFO(L"Config file found on volume %d.\n", index);
> config_volumes[rootCount] = index;
> rootCount++;
> status = close_cfg_file(volumes[index].root, fh);
> if (EFI_ERROR(status)) {
> - Print(L"Could not close config file on "
> + ERROR(L"Could not close config file on "
> L"partition %d.\n",
> index);
> }
> }
> }
> *numHandles = rootCount;
> - Print(L"%d config partitions detected.\n", rootCount);
> + INFO(L"%d config partitions detected.\n", rootCount);
> return EFI_SUCCESS;
> }
>
> @@ -64,7 +64,7 @@ UINTN filter_cfg_parts(UINTN *config_volumes, UINTN
> numHandles)
> {
> BOOLEAN use_envs_on_bootmedium_only = FALSE;
>
> - Print(L"Config filter: \n");
> + INFO(L"Config filter: \n");
> for (UINTN index = 0; index < numHandles; index++) {
> VOLUME_DESC *v = &volumes[config_volumes[index]];
>
> @@ -78,7 +78,7 @@ UINTN filter_cfg_parts(UINTN *config_volumes, UINTN
> numHandles)
> return numHandles;
> }
>
> - Print(L"Booting with environments from boot medium only.\n");
> + INFO(L"Booting with environments from boot medium only.\n");
> UINTN num_sorted = 0;
> for (UINTN j = 0; j < numHandles; j++) {
> UINTN cvi = config_volumes[j];
> @@ -88,7 +88,7 @@ UINTN filter_cfg_parts(UINTN *config_volumes, UINTN
> numHandles)
> swap_uintn(&config_volumes[j],
> &config_volumes[num_sorted++]);
> } else {
> - Print(L"Ignoring config on volume #%d\n", cvi);
> + WARNING(L"Ignoring config on volume #%d\n", cvi);
> }
> }
>
> diff --git a/include/utils.h b/include/utils.h
> index 8ce5ac6..72a7cbb 100644
> --- a/include/utils.h
> +++ b/include/utils.h
> @@ -40,6 +40,24 @@ EFI_DEVICE_PATH *FileDevicePathFromConfig(EFI_HANDLE
> device,
> CHAR16 *payloadpath);
> CHAR16 *GetBootMediumPath(CHAR16 *input);
> BOOLEAN IsOnBootMedium(EFI_DEVICE_PATH *dp);
> -VOID Color(EFI_SYSTEM_TABLE *system_table, char fgcolor, char bgcolor);
> +
> +VOID PrintC(const UINT8 color, const CHAR16 *fmt, ...);
> +#define ERROR(fmt, ...)
> \
> + do { \
> + PrintC(EFI_LIGHTRED, L"ERROR: "); \
> + PrintC(EFI_LIGHTGRAY, fmt, ##__VA_ARGS__); \
> + } while (0)
> +
> +#define WARNING(fmt, ...)
> \
> + do { \
> + PrintC(EFI_YELLOW, L"WARNING: "); \
> + PrintC(EFI_LIGHTGRAY, fmt, ##__VA_ARGS__); \
> + } while (0)
> +
> +#define INFO(fmt, ...)
> \
> + do { \
> + PrintC(EFI_WHITE, L"> "); \
> + PrintC(EFI_LIGHTGRAY, fmt, ##__VA_ARGS__); \
> + } while (0)
>
> #endif // __H_UTILS__
> diff --git a/main.c b/main.c
> index 9fe8bbe..320e4dc 100644
> --- a/main.c
> +++ b/main.c
> @@ -114,9 +114,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> this_image = image_handle;
> InitializeLib(this_image, system_table);
>
> - Color(system_table, 3, 0);
> - Print(L"\nEFI Boot Guard %s\n", L""EFIBOOTGUARD_VERSION);
> - Color(system_table, 7, 0);
> + PrintC(EFI_CYAN, L"\nEFI Boot Guard %s\n", L"" EFIBOOTGUARD_VERSION);
>
> status =
> uefi_call_wrapper(BS->OpenProtocol, 6, this_image,
> @@ -131,7 +129,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> tmp = DevicePathToStr(DevicePathFromHandle(loaded_image->DeviceHandle));
> boot_medium_path = GetBootMediumPath(tmp);
> mfree(tmp);
> - Print(L"Boot medium: %s\n", boot_medium_path);
> + INFO(L"Boot medium: %s\n", boot_medium_path);
>
> status = get_volumes(&volumes, &volume_count);
> if (EFI_ERROR(status)) {
> @@ -139,7 +137,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> status);
> }
>
> - Print(L"Loading configuration...\n");
> + INFO(L"Loading configuration...\n");
>
> bg_status = load_config(&bg_loader_params);
> if (BG_ERROR(bg_status)) {
> @@ -151,7 +149,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> EFI_ABORTED);
> break;
> case BG_CONFIG_PARTIALLY_CORRUPTED:
> - Print(L"Config is partially corrupted. Please check.\n"
> + WARNING(L"Config is partially corrupted. Please
> check.\n"
> L"efibootguard will try to boot.\n");
> break;
> default:
> @@ -170,7 +168,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> }
>
> if (bg_loader_params.timeout == 0) {
> - Print(L"Watchdog is disabled.\n");
> + WARNING(L"Watchdog is disabled.\n");
> } else {
> status = scan_devices(loaded_image, bg_loader_params.timeout);
> if (EFI_ERROR(status)) {
> @@ -202,7 +200,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> loaded_image->LoadOptionsSize =
> (StrLen(bg_loader_params.payload_options) + 1) * sizeof(CHAR16);
>
> - Print(L"Starting %s with watchdog set to %d seconds\n",
> + INFO(L"Starting %s with watchdog set to %d seconds ...",
> bg_loader_params.payload_path, bg_loader_params.timeout);
>
> return uefi_call_wrapper(BS->StartImage, 3, payload_handle, 0, 0);
> diff --git a/utils.c b/utils.c
> index cda0e92..cb9569d 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -16,6 +16,19 @@
> #include <bootguard.h>
> #include <utils.h>
>
> +VOID PrintC(const UINT8 color, const CHAR16 *fmt, ...)
> +{
> + INT32 attr = ST->ConOut->Mode->Attribute;
> + (VOID)uefi_call_wrapper(ST->ConOut->SetAttribute, 2, ST->ConOut, color);
> +
> + va_list args;
> + va_start(args, fmt);
> + (VOID)VPrint(fmt, args);
> + va_end(args);
> +
> + (VOID)uefi_call_wrapper(ST->ConOut->SetAttribute, 2, ST->ConOut, attr);
> +}
> +
> BOOLEAN IsOnBootMedium(EFI_DEVICE_PATH *dp)
> {
> extern CHAR16 *boot_medium_path;
> @@ -44,7 +57,7 @@ uint32_t calc_crc32(void *data, int32_t size)
>
> void __noreturn error_exit(CHAR16 *message, EFI_STATUS status)
> {
> - Print(L"%s ( %r )\n", message, status);
> + ERROR(L"%s ( %r )\n", message, status);
> uefi_call_wrapper(BS->Stall, 1, 3 * 1000 * 1000);
> uefi_call_wrapper(BS->Exit, 4, this_image, status, 0, NULL);
> unreachable();
> @@ -121,21 +134,21 @@ EFI_STATUS get_volumes(VOLUME_DESC **volumes, UINTN
> *count)
> EFI_FILE_HANDLE tmp;
>
> if (!volumes || !count) {
> - Print(L"Invalid volume enumeration.\n");
> + ERROR(L"Invalid volume enumeration.\n");
> return EFI_INVALID_PARAMETER;
> }
>
> status = uefi_call_wrapper(BS->LocateHandleBuffer, 5, ByProtocol,
> &sfspGuid, NULL, &handleCount, &handles);
> if (EFI_ERROR(status)) {
> - Print(L"Could not locate handle buffer.\n");
> + ERROR(L"Could not locate handle buffer.\n");
> return EFI_OUT_OF_RESOURCES;
> }
> - Print(L"Found %d handles for file IO\n\n", handleCount);
> + INFO(L"Found %d handles for file IO\n\n", handleCount);
>
> *volumes = (VOLUME_DESC *)mmalloc(sizeof(VOLUME_DESC) * handleCount);
> if (!*volumes) {
> - Print(L"Could not allocate memory for volume descriptors.\n");
> + ERROR(L"Could not allocate memory for volume descriptors.\n");
> return EFI_OUT_OF_RESOURCES;
> }
>
> @@ -148,7 +161,7 @@ EFI_STATUS get_volumes(VOLUME_DESC **volumes, UINTN
> *count)
> &sfspGuid, (VOID **)&fs);
> if (EFI_ERROR(status)) {
> /* skip failed handle and continue enumerating */
> - Print(L"File IO handle %d does not support "
> + ERROR(L"File IO handle %d does not support "
> L"SIMPLE_FILE_SYSTEM_PROTOCOL, skipping.\n",
> index);
> continue;
> @@ -156,14 +169,14 @@ EFI_STATUS get_volumes(VOLUME_DESC **volumes, UINTN
> *count)
> status = uefi_call_wrapper(fs->OpenVolume, 2, fs, &tmp);
> if (EFI_ERROR(status)) {
> /* skip failed handle and continue enumerating */
> - Print(L"Could not open file system for IO handle %d, "
> + ERROR(L"Could not open file system for IO handle %d, "
> L"skipping.\n",
> index);
> continue;
> }
> EFI_DEVICE_PATH *devpath = DevicePathFromHandle(handles[index]);
> if (devpath == NULL) {
> - Print(
> + ERROR(
> L"Could not get device path for config partition, "
> L"skipping.\n");
> continue;
> @@ -176,11 +189,11 @@ EFI_STATUS get_volumes(VOLUME_DESC **volumes, UINTN
> *count)
> get_volume_label((*volumes)[rootCount].root);
> (*volumes)[rootCount].fscustomlabel =
> get_volume_custom_label((*volumes)[rootCount].root);
> - Print(L"Volume %d: ", rootCount);
> + INFO(L"Volume %d: ", rootCount);
> if (IsOnBootMedium(devpath)) {
> - Print(L"(On boot medium) ");
> + PrintC(EFI_LIGHTGRAY, L"(On boot medium) ");
> }
> - Print(L"%s, LABEL=%s, CLABEL=%s\n",
> + PrintC(EFI_LIGHTGRAY, L"%s, LABEL=%s, CLABEL=%s\n",
> devpathstr, (*volumes)[rootCount].fslabel,
> (*volumes)[rootCount].fscustomlabel);
>
> @@ -198,20 +211,20 @@ EFI_STATUS close_volumes(VOLUME_DESC *volumes, UINTN
> count)
> UINTN i;
>
> if (!volumes) {
> - Print(L"Invalid parameter for closing volumes.\n");
> + ERROR(L"Invalid parameter for closing volumes.\n");
> return EFI_INVALID_PARAMETER;
> }
> for (i = 0; i < count; i++) {
> EFI_STATUS status;
>
> if (!volumes[i].root) {
> - Print(L"Error, invalid handle for volume %d.\n", i);
> + ERROR(L"Invalid handle for volume %d.\n", i);
> result = EFI_INVALID_PARAMETER;
> }
> status = uefi_call_wrapper(volumes[i].root->Close, 1,
> volumes[i].root);
> if (EFI_ERROR(status)) {
> - Print(L"Could not close volume %d.\n", i);
> + ERROR(L"Could not close volume %d.\n", i);
> result = EFI_DEVICE_ERROR;
> }
> }
> @@ -277,7 +290,7 @@ EFI_DEVICE_PATH *FileDevicePathFromConfig(EFI_HANDLE
> device,
>
> StrCpy(fullpath, pathprefix);
> StrCat(fullpath, payloadpath + prefixlen + 3);
> - Print(L"Full path for kernel is: %s\n", fullpath);
> + INFO(L"Full path for kernel is: %s\n", fullpath);
>
> mfree(fullpath);
> mfree(pathprefix);
> @@ -317,9 +330,3 @@ CHAR16 *GetBootMediumPath(CHAR16 *input)
>
> return dst;
> }
> -
> -VOID Color(EFI_SYSTEM_TABLE *system_table, char fgcolor, char bgcolor)
> -{
> - SIMPLE_TEXT_OUTPUT_INTERFACE *con = system_table->ConOut;
> - (VOID)uefi_call_wrapper(con->SetAttribute, 3, con, (bgcolor << 8) |
> fgcolor);
> -}
>
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups "EFI
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/efibootguard-dev/dd0262ec-525d-5ff4-65ec-cacb27a9f62a%40siemens.com.
