On 03/13/2014 08:49 PM, Stephen Warren wrote:
On 03/13/2014 11:28 AM, Przemyslaw Marczak wrote:
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
@@ -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 ret has nothing to do with it; there's no code in this function
which prints messges based on ret.
The code should be:
e = getenv(s);
if (e == NULL) {
printf("Env var '%s' not set; using random UUID\n", str);
// generate random UUID here
if (UUID generation failed) {
printf("ERROR generating random UUID\n");
return -1;
}
Take a note that gen_rand_uuid_str() doesn't return any error. So this
error is not true.
}
With the code in your patch, the following happens:
* Env var is set: Nothing printed.
And env var is not changed - and this is as before.
* Env var not set, but problem during UUID generation: "%s unset. " is
printed without a \n at the end. The error does get propagated back tot
he caller, which might print a message with \n at the end, but you
shouldn't place that kind of requirement on the caller. Rather, this
function should be fully self-contained.
* Env var set, and UUID generated OK: "%s unset. Setting to random.\n"
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.
I don't think convenience of coding or the size of the patch is
justification for writing values to the user's environment when they
didn't ask for it. What if they run saveenv after executing this
function, yet they specifically want the environment variables unset, so
that a random UUID is generated each time this function/command is run?
Actually I don't understand what is the problem. What is the difference
when user set manually $uuid_gpt_* or use generated by gpt command if
next he write "save", in both situations variables are saved. I don't
think it is a problem.
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
