Hello,
Sorry for silent, but I've had some other work.
I agree with yours previous comments and those will apply to v3 but I
don't agree with few comments to this patch.
On 03/10/2014 06:44 PM, Stephen Warren wrote:
On 03/05/2014 09:45 AM, Przemyslaw Marczak wrote:
Changes:
- randomly generate each partition uuid if undefined
- print info about generated uuid
- save environment on gpt write success
- update doc/README.gpt
diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c
-static char extract_env(const char *str, char **env)
+static int extract_env(const char *str, char **env)
{
+ int ret = -1;
char *e, *s;
+ char uuid_str[37];
if (!str || strlen(str) < 4)
return -1;
@@ -43,16 +45,25 @@ static char extract_env(const char *str, char **env)
memset(s + strlen(s) - 1, '\0', 1);
memmove(s, s + 2, strlen(s) - 1);
e = getenv(s);
- free(s);
if (e == NULL) {
- printf("Environmental '%s' not set\n", str);
- return -1; /* env not set */
+ printf("%s unset. ", str);
+ gen_rand_uuid_str(uuid_str);
+ setenv(s, uuid_str);
+
In this place ret is "-1".
+ e = getenv(s);
+ if (e) {
+ puts("Setting to random.\n");
Shouldn't this be printed right after the "if (e == NULL)" check above?
That's where the decision is made to generate a random UUID.
Here, "if (!e)", the code should return an error.
If (!e) then ret is still "-1".
If (e) then ret = 0 and proper info is printed.
But, I still don't like changing the environment. Why can't the above
few lines be:
+ gen_rand_uuid_str(uuid_str);
+ e = uuid_str;
Such solution needs more code rewriting and breaking some existing
cmd_gpt design. "e" is used outside this function but uuid_str is local
here. I don't like to make it static.
Using getenv and return its pointer will work the same as previous.
Please note that variables set by user are not overwritten here so this
code will only set null uuid env variables. Moreover user can see after
gpt command that environment is the same with mmc part shows, I think it
is useful instead of situation when uuid is set but not present in
environment.
diff --git a/doc/README.gpt b/doc/README.gpt
"uuid" program is recommended to generate UUID string. Moreover it can decode
(-d switch) passed in UUID string. It can be used to generate partitions UUID
passed to u-boot environment variables.
+If each partition "uuid" no exists then it will be randomly generated.
"If each" means "if all of them", implying that it's an all-or-nothing
solution, and the random generation only happens of none of the UUIDs
were supplied, not on a UUID-by-UUID basis. So, s/each/a/ or s/each/any/.
Ok :)
Thanks
--
Przemyslaw Marczak
Samsung R&D 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
