diff mbox series

[1/1] efi_loader: fix 'efidebug bootorder'

Message ID 20200429192301.28751-1-xypron.glpk@gmx.de
State Accepted
Commit f9f5f92bc54b035223e447bc5740544efd0569d9
Headers show
Series [1/1] efi_loader: fix 'efidebug bootorder' | expand

Commit Message

Heinrich Schuchardt April 29, 2020, 7:23 p.m. UTC 
* don't copy GUIDs for no reason
* shorten print format strings by using variable names
* don't use the run-time table to access exported functions
* check the result of malloc() (fixes Coverity CID 300331)

Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
 cmd/efidebug.c | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

--
2.26.2

Comments

AKASHI Takahiro May 7, 2020, 6:46 a.m. UTC | #1 
On Wed, Apr 29, 2020 at 09:23:01PM +0200, Heinrich Schuchardt wrote:
> * don't copy GUIDs for no reason
> * shorten print format strings by using variable names
> * don't use the run-time table to access exported functions

I don't much care about those changes, but my initial intent
was that any efi-related command be implemented as if it were
an efi application so that it can be converted with less efforts.

-Takahiro Akashi

> * check the result of malloc() (fixes Coverity CID 300331)
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  cmd/efidebug.c | 47 ++++++++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index 53cb3cbfaf..d4030fee64 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -852,8 +852,7 @@ static int do_efi_boot_dump(cmd_tbl_t *cmdtp, int flag,
>   */
>  static int show_efi_boot_order(void)
>  {
> -	efi_guid_t guid;
> -	u16 *bootorder = NULL;
> +	u16 *bootorder;
>  	efi_uintn_t size;
>  	int num, i;
>  	char var_name[9];
> @@ -864,20 +863,25 @@ static int show_efi_boot_order(void)
>  	size_t label_len16, label_len;
>  	efi_status_t ret;
> 
> -	guid = efi_global_variable_guid;
>  	size = 0;
> -	ret = EFI_CALL(RT->get_variable(L"BootOrder", &guid, NULL, &size,
> -					NULL));
> -	if (ret == EFI_BUFFER_TOO_SMALL) {
> -		bootorder = malloc(size);
> -		ret = EFI_CALL(RT->get_variable(L"BootOrder", &guid, NULL,
> -						&size, bootorder));
> +	ret = EFI_CALL(RT->get_variable(L"BootOrder", &efi_global_variable_guid,
> +					NULL, &size, NULL));
> +	if (ret != EFI_BUFFER_TOO_SMALL) {
> +		if (ret == EFI_NOT_FOUND) {
> +			printf("BootOrder not defined\n");
> +			return CMD_RET_SUCCESS;
> +		} else {
> +			return CMD_RET_FAILURE;
> +		}
>  	}
> -	if (ret == EFI_NOT_FOUND) {
> -		printf("BootOrder not defined\n");
> -		ret = CMD_RET_SUCCESS;
> -		goto out;
> -	} else if (ret != EFI_SUCCESS) {
> +	bootorder = malloc(size);
> +	if (!bootorder) {
> +		printf("ERROR: Out of memory\n");
> +		return CMD_RET_FAILURE;
> +	}
> +	ret = EFI_CALL(efi_get_variable(L"BootOrder", &efi_global_variable_guid,
> +					NULL, &size, bootorder));
> +	if (ret != EFI_SUCCESS) {
>  		ret = CMD_RET_FAILURE;
>  		goto out;
>  	}
> @@ -889,11 +893,11 @@ static int show_efi_boot_order(void)
>  		utf8_utf16_strncpy(&p16, var_name, 9);
> 
>  		size = 0;
> -		ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size,
> -						NULL));
> +		ret = EFI_CALL(efi_get_variable(var_name16,
> +						&efi_global_variable_guid, NULL,
> +						&size, NULL));
>  		if (ret != EFI_BUFFER_TOO_SMALL) {
> -			printf("%2d: Boot%04X: (not defined)\n",
> -			       i + 1, bootorder[i]);
> +			printf("%2d: %s: (not defined)\n", i + 1, var_name);
>  			continue;
>  		}
> 
> @@ -902,8 +906,9 @@ static int show_efi_boot_order(void)
>  			ret = CMD_RET_FAILURE;
>  			goto out;
>  		}
> -		ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size,
> -						data));
> +		ret = EFI_CALL(efi_get_variable(var_name16,
> +						&efi_global_variable_guid, NULL,
> +						&size, data));
>  		if (ret != EFI_SUCCESS) {
>  			free(data);
>  			ret = CMD_RET_FAILURE;
> @@ -922,7 +927,7 @@ static int show_efi_boot_order(void)
>  		}
>  		p = label;
>  		utf16_utf8_strncpy(&p, lo.label, label_len16);
> -		printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
> +		printf("%2d: %s: %s\n", i + 1, var_name, label);
>  		free(label);
> 
>  		free(data);
> --
> 2.26.2
>
Heinrich Schuchardt May 7, 2020, 8:33 p.m. UTC | #2 
On 5/7/20 8:46 AM, AKASHI Takahiro wrote:
> On Wed, Apr 29, 2020 at 09:23:01PM +0200, Heinrich Schuchardt wrote:
>> * don't copy GUIDs for no reason
>> * shorten print format strings by using variable names
>> * don't use the run-time table to access exported functions
>
> I don't much care about those changes, but my initial intent
> was that any efi-related command be implemented as if it were
> an efi application so that it can be converted with less efforts.

Tom and other maintainers want to minimize the binary size of U-Boot.
And these changes save a few bytes.

Best regards

Heinrich

>
> -Takahiro Akashi
>
>> * check the result of malloc() (fixes Coverity CID 300331)
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>  cmd/efidebug.c | 47 ++++++++++++++++++++++++++---------------------
>>  1 file changed, 26 insertions(+), 21 deletions(-)
>>
>> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>> index 53cb3cbfaf..d4030fee64 100644
>> --- a/cmd/efidebug.c
>> +++ b/cmd/efidebug.c
>> @@ -852,8 +852,7 @@ static int do_efi_boot_dump(cmd_tbl_t *cmdtp, int flag,
>>   */
>>  static int show_efi_boot_order(void)
>>  {
>> -	efi_guid_t guid;
>> -	u16 *bootorder = NULL;
>> +	u16 *bootorder;
>>  	efi_uintn_t size;
>>  	int num, i;
>>  	char var_name[9];
>> @@ -864,20 +863,25 @@ static int show_efi_boot_order(void)
>>  	size_t label_len16, label_len;
>>  	efi_status_t ret;
>>
>> -	guid = efi_global_variable_guid;
>>  	size = 0;
>> -	ret = EFI_CALL(RT->get_variable(L"BootOrder", &guid, NULL, &size,
>> -					NULL));
>> -	if (ret == EFI_BUFFER_TOO_SMALL) {
>> -		bootorder = malloc(size);
>> -		ret = EFI_CALL(RT->get_variable(L"BootOrder", &guid, NULL,
>> -						&size, bootorder));
>> +	ret = EFI_CALL(RT->get_variable(L"BootOrder", &efi_global_variable_guid,
>> +					NULL, &size, NULL));
>> +	if (ret != EFI_BUFFER_TOO_SMALL) {
>> +		if (ret == EFI_NOT_FOUND) {
>> +			printf("BootOrder not defined\n");
>> +			return CMD_RET_SUCCESS;
>> +		} else {
>> +			return CMD_RET_FAILURE;
>> +		}
>>  	}
>> -	if (ret == EFI_NOT_FOUND) {
>> -		printf("BootOrder not defined\n");
>> -		ret = CMD_RET_SUCCESS;
>> -		goto out;
>> -	} else if (ret != EFI_SUCCESS) {
>> +	bootorder = malloc(size);
>> +	if (!bootorder) {
>> +		printf("ERROR: Out of memory\n");
>> +		return CMD_RET_FAILURE;
>> +	}
>> +	ret = EFI_CALL(efi_get_variable(L"BootOrder", &efi_global_variable_guid,
>> +					NULL, &size, bootorder));
>> +	if (ret != EFI_SUCCESS) {
>>  		ret = CMD_RET_FAILURE;
>>  		goto out;
>>  	}
>> @@ -889,11 +893,11 @@ static int show_efi_boot_order(void)
>>  		utf8_utf16_strncpy(&p16, var_name, 9);
>>
>>  		size = 0;
>> -		ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size,
>> -						NULL));
>> +		ret = EFI_CALL(efi_get_variable(var_name16,
>> +						&efi_global_variable_guid, NULL,
>> +						&size, NULL));
>>  		if (ret != EFI_BUFFER_TOO_SMALL) {
>> -			printf("%2d: Boot%04X: (not defined)\n",
>> -			       i + 1, bootorder[i]);
>> +			printf("%2d: %s: (not defined)\n", i + 1, var_name);
>>  			continue;
>>  		}
>>
>> @@ -902,8 +906,9 @@ static int show_efi_boot_order(void)
>>  			ret = CMD_RET_FAILURE;
>>  			goto out;
>>  		}
>> -		ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size,
>> -						data));
>> +		ret = EFI_CALL(efi_get_variable(var_name16,
>> +						&efi_global_variable_guid, NULL,
>> +						&size, data));
>>  		if (ret != EFI_SUCCESS) {
>>  			free(data);
>>  			ret = CMD_RET_FAILURE;
>> @@ -922,7 +927,7 @@ static int show_efi_boot_order(void)
>>  		}
>>  		p = label;
>>  		utf16_utf8_strncpy(&p, lo.label, label_len16);
>> -		printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
>> +		printf("%2d: %s: %s\n", i + 1, var_name, label);
>>  		free(label);
>>
>>  		free(data);
>> --
>> 2.26.2
>>
diff mbox series

Patch

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 53cb3cbfaf..d4030fee64 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -852,8 +852,7 @@  static int do_efi_boot_dump(cmd_tbl_t *cmdtp, int flag,
  */
 static int show_efi_boot_order(void)
 {
-	efi_guid_t guid;
-	u16 *bootorder = NULL;
+	u16 *bootorder;
 	efi_uintn_t size;
 	int num, i;
 	char var_name[9];
@@ -864,20 +863,25 @@  static int show_efi_boot_order(void)
 	size_t label_len16, label_len;
 	efi_status_t ret;

-	guid = efi_global_variable_guid;
 	size = 0;
-	ret = EFI_CALL(RT->get_variable(L"BootOrder", &guid, NULL, &size,
-					NULL));
-	if (ret == EFI_BUFFER_TOO_SMALL) {
-		bootorder = malloc(size);
-		ret = EFI_CALL(RT->get_variable(L"BootOrder", &guid, NULL,
-						&size, bootorder));
+	ret = EFI_CALL(RT->get_variable(L"BootOrder", &efi_global_variable_guid,
+					NULL, &size, NULL));
+	if (ret != EFI_BUFFER_TOO_SMALL) {
+		if (ret == EFI_NOT_FOUND) {
+			printf("BootOrder not defined\n");
+			return CMD_RET_SUCCESS;
+		} else {
+			return CMD_RET_FAILURE;
+		}
 	}
-	if (ret == EFI_NOT_FOUND) {
-		printf("BootOrder not defined\n");
-		ret = CMD_RET_SUCCESS;
-		goto out;
-	} else if (ret != EFI_SUCCESS) {
+	bootorder = malloc(size);
+	if (!bootorder) {
+		printf("ERROR: Out of memory\n");
+		return CMD_RET_FAILURE;
+	}
+	ret = EFI_CALL(efi_get_variable(L"BootOrder", &efi_global_variable_guid,
+					NULL, &size, bootorder));
+	if (ret != EFI_SUCCESS) {
 		ret = CMD_RET_FAILURE;
 		goto out;
 	}
@@ -889,11 +893,11 @@  static int show_efi_boot_order(void)
 		utf8_utf16_strncpy(&p16, var_name, 9);

 		size = 0;
-		ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size,
-						NULL));
+		ret = EFI_CALL(efi_get_variable(var_name16,
+						&efi_global_variable_guid, NULL,
+						&size, NULL));
 		if (ret != EFI_BUFFER_TOO_SMALL) {
-			printf("%2d: Boot%04X: (not defined)\n",
-			       i + 1, bootorder[i]);
+			printf("%2d: %s: (not defined)\n", i + 1, var_name);
 			continue;
 		}

@@ -902,8 +906,9 @@  static int show_efi_boot_order(void)
 			ret = CMD_RET_FAILURE;
 			goto out;
 		}
-		ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size,
-						data));
+		ret = EFI_CALL(efi_get_variable(var_name16,
+						&efi_global_variable_guid, NULL,
+						&size, data));
 		if (ret != EFI_SUCCESS) {
 			free(data);
 			ret = CMD_RET_FAILURE;
@@ -922,7 +927,7 @@  static int show_efi_boot_order(void)
 		}
 		p = label;
 		utf16_utf8_strncpy(&p, lo.label, label_len16);
-		printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
+		printf("%2d: %s: %s\n", i + 1, var_name, label);
 		free(label);

 		free(data);