[U-Boot] [PATCH] gpt: support random UUIDs without setting environment variables

[Przemyslaw Marczak]

Hello Rob,

Sorry for delay.

On 01/26/2015 04:44 PM, Rob Herring wrote:
> Currently, an environment variable must be used to store the randomly
> generated UUID for each partition. This is not necessary, so make storing
> the UUID optional. Now passing uuid_disk and uuid are optional when random
> UUIDs are enabled.
>
> Signed-off-by: Rob Herring <robh at kernel.org>
> ---
>   common/cmd_gpt.c | 48 ++++++++++++++++++++++++++++++------------------
>   doc/README.gpt   |  8 +++++---
>   2 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
> index 75df3fe..c56fe15 100644
> --- a/common/cmd_gpt.c
> +++ b/common/cmd_gpt.c
> @@ -154,17 +154,24 @@ static int set_gpt_info(block_dev_desc_t *dev_desc,
>
>   	/* extract disk guid */
>   	s = str;
> -	tok = strsep(&s, ";");
> -	val = extract_val(tok, "uuid_disk");
> +	val = extract_val(str, "uuid_disk");
>   	if (!val) {

The code below is not required, since the same thing is done inside the 
extract_env() function.

> +#ifdef CONFIG_RANDOM_UUID
> +		*str_disk_guid = malloc(UUID_STR_LEN + 1);
> +		gen_rand_uuid_str(*str_disk_guid, UUID_STR_FORMAT_STD);
> +#else
>   		free(str);
>   		return -2;
> +#endif
> +	} else {
> +		val = strsep(&val, ";");
> +		if (extract_env(val, &p))
> +			p = val;
> +		*str_disk_guid = strdup(p);
> +		free(val);
> +		/* Move s to first partition */
> +		strsep(&s, ";");
>   	}
> -	if (extract_env(val, &p))
> -		p = val;
> -	*str_disk_guid = strdup(p);
> -	free(val);
> -
>   	if (strlen(s) == 0)
>   		return -3;
>
> @@ -192,20 +199,25 @@ static int set_gpt_info(block_dev_desc_t *dev_desc,
>
>   		/* uuid */
>   		val = extract_val(tok, "uuid");
> -		if (!val) { /* 'uuid' is mandatory */
> -			errno = -4;
> -			goto err;
> -		}
> -		if (extract_env(val, &p))
> -			p = val;
> -		if (strlen(p) >= sizeof(parts[i].uuid)) {
> -			printf("Wrong uuid format for partition %d\n", i);
> +		if (!val) {

The same in this place - code duplication.

> +			/* 'uuid' is optional if random uuid's are enabled */
> +#ifdef CONFIG_RANDOM_UUID
> +			gen_rand_uuid_str(parts[i].uuid, UUID_STR_FORMAT_STD);
> +#else
>   			errno = -4;
>   			goto err;
> +#endif
> +		} else {
> +			if (extract_env(val, &p))
> +				p = val;
> +			if (strlen(p) >= sizeof(parts[i].uuid)) {
> +				printf("Wrong uuid format for partition %d\n", i);
> +				errno = -4;
> +				goto err;
> +			}
> +			strcpy((char *)parts[i].uuid, p);
> +			free(val);
>   		}
> -		strcpy((char *)parts[i].uuid, p);
> -		free(val);
> -
>   		/* name */
>   		val = extract_val(tok, "name");
>   		if (!val) { /* name is mandatory */
> diff --git a/doc/README.gpt b/doc/README.gpt
> index ec0156d..59fdeeb 100644
> --- a/doc/README.gpt
> +++ b/doc/README.gpt
> @@ -157,11 +157,13 @@ To restore GUID partition table one needs to:
>        "partitions=uuid_disk=${uuid_gpt_disk};name=${uboot_name},
>   	size=${uboot_size},uuid=${uboot_uuid};"
>
> -   Fields 'name', 'size' and 'uuid' are mandatory for every partition.
> +   The fields 'name' and 'size' are mandatory for every partition.
>      The field 'start' is optional.
>
> -   option: CONFIG_RANDOM_UUID
> -   If any partition "UUID" no exists then it is randomly generated.
> +   The fields 'uuid' and 'uuid_disk' are optional if CONFIG_RANDOM_UUID is
> +   enabled. A random uuid will be used if omitted or they point to an empty/
> +   non-existent environment variable. The environment variable will be set to
> +   the generated UUID.

The things from the above comment are implemented at present in mainline.

>
>   2. Define 'CONFIG_EFI_PARTITION' and 'CONFIG_CMD_GPT'
>
>


If you want drop saving the uuid to env, then you can do it by modify 
the extract_env() function (diff):

+#ifdef CONFIG_RANDOM_UUID_SKIP_SETENV
+               e = strdup(uuid_str);
+#else
                 setenv(s, uuid_str);
-
                 e = getenv(s);
+#endif


I would prefer introduce the new config like *SKIP_SETENV*, rather than 
drop this feature at all, since somebody could use it.

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com