[PATCH] efidebug: avoid 'dfu_alt_info not defined' message
If variable dfu_alt_info is not defined duplicate messages are displayed.
=> efidebug boot dump Scanning disk mmc2.blk... Scanning disk mmc1.blk... Scanning disk mmc0.blk... Found 3 disks No EFI system partition "dfu_alt_info" env variable not defined! Probably dfu_alt_info not defined "dfu_alt_info" env variable not defined! Probably dfu_alt_info not defined
Remove the 'Probably dfu_alt_info not defined' message. Instead write a warning if the variable contains no entities.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- lib/efi_loader/efi_firmware.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index a1b88dbfc2..519a47267c 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -128,8 +128,11 @@ static efi_status_t efi_get_dfu_info( size_t names_len, total_size; int dfu_num, i; u16 *name, *next; + int ret;
- dfu_init_env_entities(NULL, NULL); + ret = dfu_init_env_entities(NULL, NULL); + if (ret) + return EFI_SUCCESS;
names_len = 0; dfu_num = 0; @@ -138,7 +141,7 @@ static efi_status_t efi_get_dfu_info( dfu_num++; } if (!dfu_num) { - log_warning("Probably dfu_alt_info not defined\n"); + log_warning("No entities in dfu_alt_info\n"); *image_info_size = 0; dfu_free_entities();
On Sat, Jan 15, 2022 at 02:18:21AM +0100, Heinrich Schuchardt wrote:
If variable dfu_alt_info is not defined duplicate messages are displayed.
=> efidebug boot dump Scanning disk mmc2.blk... Scanning disk mmc1.blk... Scanning disk mmc0.blk... Found 3 disks No EFI system partition "dfu_alt_info" env variable not defined! Probably dfu_alt_info not defined "dfu_alt_info" env variable not defined! Probably dfu_alt_info not definedRemove the 'Probably dfu_alt_info not defined' message. Instead write a warning if the variable contains no entities.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_firmware.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index a1b88dbfc2..519a47267c 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -128,8 +128,11 @@ static efi_status_t efi_get_dfu_info( size_t names_len, total_size; int dfu_num, i; u16 *name, *next;
- int ret;
- dfu_init_env_entities(NULL, NULL);
- ret = dfu_init_env_entities(NULL, NULL);
- if (ret)
return EFI_SUCCESS;
Do you want to return EFI_SUCCESS here? It is mandatory that "dfu_alt_info" be defined so that the current FMP drivers should work. So if the variable is not defined, I think, efi_firmware_[fit|raw]_get_image_info() should fail. Even if it returns EFI_SUCCESS, descriptor_count should be set to zero.
-Takahiro Akashi
names_len = 0; dfu_num = 0; @@ -138,7 +141,7 @@ static efi_status_t efi_get_dfu_info( dfu_num++; } if (!dfu_num) {
log_warning("Probably dfu_alt_info not defined\n");
log_warning("No entities in dfu_alt_info\n");-- 2.33.1
On 1/17/22 01:45, AKASHI Takahiro wrote:
On Sat, Jan 15, 2022 at 02:18:21AM +0100, Heinrich Schuchardt wrote:
If variable dfu_alt_info is not defined duplicate messages are displayed.
=> efidebug boot dump Scanning disk mmc2.blk... Scanning disk mmc1.blk... Scanning disk mmc0.blk... Found 3 disks No EFI system partition "dfu_alt_info" env variable not defined! Probably dfu_alt_info not defined "dfu_alt_info" env variable not defined! Probably dfu_alt_info not definedRemove the 'Probably dfu_alt_info not defined' message. Instead write a warning if the variable contains no entities.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_firmware.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index a1b88dbfc2..519a47267c 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -128,8 +128,11 @@ static efi_status_t efi_get_dfu_info( size_t names_len, total_size; int dfu_num, i; u16 *name, *next;
- int ret;
- dfu_init_env_entities(NULL, NULL);
- ret = dfu_init_env_entities(NULL, NULL);
- if (ret)
return EFI_SUCCESS;Do you want to return EFI_SUCCESS here? It is mandatory that "dfu_alt_info" be defined so that the current FMP drivers should work. So if the variable is not defined, I think, efi_firmware_[fit|raw]_get_image_info() should fail. Even if it returns EFI_SUCCESS, descriptor_count should be set to zero.
This patch did not change the return value. It only removed superfluous messages that I saw everytime I used the sandbox.
For dfu_num == 0 we are returning EFI_SUCCESS. If the number is 0 due to the content of dfu_alt_info or due to its absence, should not make a difference in the return code. Writing a message if dfu_alt_info exists but contains no entity seems reasonable.
If you think EFI_SUCCESS is not correct in both cases, please, send a followup patch for review.
Best regards
Heinrich
-Takahiro Akashi
names_len = 0; dfu_num = 0; @@ -138,7 +141,7 @@ static efi_status_t efi_get_dfu_info( dfu_num++; } if (!dfu_num) {
log_warning("Probably dfu_alt_info not defined\n");
log_warning("No entities in dfu_alt_info\n");-- 2.33.1
On Mon, Jan 17, 2022 at 01:33:51PM +0100, Heinrich Schuchardt wrote:
On 1/17/22 01:45, AKASHI Takahiro wrote:
On Sat, Jan 15, 2022 at 02:18:21AM +0100, Heinrich Schuchardt wrote:
If variable dfu_alt_info is not defined duplicate messages are displayed.
=> efidebug boot dump Scanning disk mmc2.blk... Scanning disk mmc1.blk... Scanning disk mmc0.blk... Found 3 disks No EFI system partition "dfu_alt_info" env variable not defined! Probably dfu_alt_info not defined "dfu_alt_info" env variable not defined! Probably dfu_alt_info not definedRemove the 'Probably dfu_alt_info not defined' message. Instead write a warning if the variable contains no entities.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_firmware.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index a1b88dbfc2..519a47267c 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -128,8 +128,11 @@ static efi_status_t efi_get_dfu_info( size_t names_len, total_size; int dfu_num, i; u16 *name, *next;
- int ret;
- dfu_init_env_entities(NULL, NULL);
- ret = dfu_init_env_entities(NULL, NULL);
- if (ret)
return EFI_SUCCESS;Do you want to return EFI_SUCCESS here? It is mandatory that "dfu_alt_info" be defined so that the current FMP drivers should work. So if the variable is not defined, I think, efi_firmware_[fit|raw]_get_image_info() should fail. Even if it returns EFI_SUCCESS, descriptor_count should be set to zero.
This patch did not change the return value. It only removed superfluous messages that I saw everytime I used the sandbox.
The return value is the same, but
For dfu_num == 0 we are returning EFI_SUCCESS. If the number is 0 due to the content of dfu_alt_info or due to its absence, should not make a difference in the return code. Writing a message if dfu_alt_info exists but contains no entity seems reasonable.
With your patch, the value of "descriptor_count", as well as the other arguments, will remain uncertain even we see EFI_SUCCESS at return.
Since dfu_init_env_entities() returns -EINVAL in case "dfu_alt_info" is not defined, efi_get_dfu_info(), hence efi_firmware_[fit|raw]_get_image_info(), returns immediately without setting any value to "descriptor_count".
Have you ever tested your code?
If you think EFI_SUCCESS is not correct in both cases, please, send a followup patch for review.
NAK. We are discussing the issue against your patch. Why not do so in this thread?
-Takahiro Akashi
Best regards
Heinrich
-Takahiro Akashi
names_len = 0; dfu_num = 0; @@ -138,7 +141,7 @@ static efi_status_t efi_get_dfu_info( dfu_num++; } if (!dfu_num) {
log_warning("Probably dfu_alt_info not defined\n");
log_warning("No entities in dfu_alt_info\n");-- 2.33.1
participants (2)
-
AKASHI Takahiro -
Heinrich Schuchardt