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

2015-02-23 Thread Przemyslaw Marczak 

Hello,

On 02/22/2015 11:46 PM, Rob Herring wrote:

On Wed, Feb 18, 2015 at 11:48 AM, Przemyslaw Marczak
p.marc...@samsung.com wrote:

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 r...@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.


Okay.





+#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.


Partially yes, but as presently worded it did not make sense to me. It
took me some time to figure out I had to define env variables for each
uuid. So I expanded the text so the next person is not scratching
their head to get this to work.



I see, the text which you add is clear for me.





   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

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

2015-02-22 Thread Rob Herring 
On Wed, Feb 18, 2015 at 11:48 AM, Przemyslaw Marczak
p.marc...@samsung.com wrote:
 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 r...@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.

Okay.



 +#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.

Partially yes, but as presently worded it did not make sense to me. It
took me some time to figure out I had to define env variables for each
uuid. So I expanded the text so the next person is not scratching
their head to get this to work.



   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

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

2015-02-18 Thread 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 r...@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 RD Institute Poland
Samsung Electronics
p.marc...@samsung.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-01-26 Thread Rob Herring 
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 r...@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) {
+#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) {
+   /* '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.
 
 2. Define 'CONFIG_EFI_PARTITION' and 'CONFIG_CMD_GPT'
 
-- 
2.1.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot