[U-Boot] [PATCH] cmd/gpt: Address error cases during gpt rename more correctly

[Simon Goldschmidt]

+ Jordy, who just found a bug here...

Am 08.11.2019 um 17:24 schrieb Tom Rini:
> New analysis by the tool has shown that we have some cases where we
> weren't handling the error exit condition correctly.  When we ran into
> the ENOMEM case we wouldn't exit the function and thus incorrect things
> could happen.  Rework the unwinding such that we don't need a helper
> function now and free what we may have allocated.
> 
> Fixes: 18030d04d25d ("GPT: fix memory leaks identified by Coverity")
> Reported-by: Coverity (CID: 275475, 275476)
> Cc: Alison Chaiken <alison at she-devel.com>
> Signed-off-by: Tom Rini <trini at konsulko.com>
> ---
>   cmd/gpt.c | 35 ++++++++---------------------------
>   1 file changed, 8 insertions(+), 27 deletions(-)
> 
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index 0c4349f4b249..2da8df60dca3 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -633,21 +633,6 @@ static int do_disk_guid(struct blk_desc *dev_desc, char * const namestr)
>   }
>   
>   #ifdef CONFIG_CMD_GPT_RENAME
> -/*
> - * There are 3 malloc() calls in set_gpt_info() and there is no info about which
> - * failed.
> - */
> -static void set_gpt_cleanup(char **str_disk_guid,
> -			    disk_partition_t **partitions)
> -{
> -#ifdef CONFIG_RANDOM_UUID
> -	if (str_disk_guid)
> -		free(str_disk_guid);
> -#endif
> -	if (partitions)
> -		free(partitions);
> -}
> -
>   static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm,
>   			       char *name1, char *name2)
>   {
> @@ -699,11 +684,7 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm,
>   			   &new_partitions, &part_count);
>   	if (ret < 0) {
>   		del_gpt_info();
> -		free(partitions_list);
> -		if (ret == -ENOMEM)
> -			set_gpt_cleanup(&str_disk_guid, &new_partitions);
> -		else
> -			goto out;
> +		goto out;
>   	}
>   
>   	if (!strcmp(subcomm, "swap")) {
> @@ -768,11 +749,7 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm,
>   	 */
>   	if (ret < 0) {
>   		del_gpt_info();
> -		free(partitions_list);
> -		if (ret == -ENOMEM)
> -			set_gpt_cleanup(&str_disk_guid, &new_partitions);
> -		else
> -			goto out;
> +		goto out;
>   	}
>   
>   	debug("Writing new partition table\n");
> @@ -797,8 +774,12 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm,
>   	print_gpt_info();
>   	del_gpt_info();
>    out:
> -	free(new_partitions);
> -	free(str_disk_guid);
> +#ifdef CONFIG_RANDOM_UUID
> +	if (str_disk_guid)

Looks good overall, but could it be required to initialize str_disk_guid 
and new_partitions to 0 to make this test here work? Because 
set_gpt_info does not always seem to set these pointers in the error 
case, so they could be left uninitialized?

Regards,
Simon

> +		free(str_disk_guid);
> +#endif
> +	if (new_partitions)
> +		free(new_partitions);
>   	free(partitions_list);
>   	return ret;
>   }
>