diff mbox

[U-Boot] Recent changes break our update scripts, because of getenv_yesno()

Message ID 4D1DB35A.3090105@emk-elektronik.de
State RFC
Headers show

Commit Message

Reinhard Meyer Dec. 31, 2010, 10:41 a.m. UTC 
On 31.12.2010 11:33, Reinhard Meyer wrote:
> Hello,
>
> in common/image.c:
>
>
> int getenv_yesno (char *var)
> {
> 	char *s = getenv (var);
> 	return (s&&  (*s == 'n')) ? 0 : 1;
> }
>
> Is that supposed to return TRUE when the env variable does NOT exist?
>
> Because each TFTP/USB/whatever load of an image will automatically start it!
>
> (and why is such a generic function in image.c?)
>
OK, digging deeper, the culprit actually is:

2010-11-28
	Mike Frysinger
	boot cmds: convert to getenv_yesno() with autostart

Example:
index c657b03..973fa21 100644 (file)

Originally, a missing env var resulted in FALSE, with the helper function it
results in TRUE!!

Reinhard

Comments

Stefano Babic Dec. 31, 2010, 10:58 a.m. UTC | #1 
On 12/31/2010 11:41 AM, Reinhard Meyer wrote:

>          /* Loading ok, check if we should attempt an auto-start */
> -       if (((s = getenv("autostart")) != NULL) && (strcmp(s,"yes") == 0)) {
> +       if (getenv_yesno("autostart")) {
>                  char *local_args[2];
>                  local_args[0] = argv[0];
>                  local_args[1] = NULL;
> 
> Originally, a missing env var resulted in FALSE, with the helper function it
> results in TRUE!!

Thanks, this explains why it happens ! We should check again for a
missing env var.

Best regards,
Stefano Babic
Wolfgang Denk Dec. 31, 2010, 11:42 a.m. UTC | #2 
Dear Reinhard Meyer,

In message <4D1DB35A.3090105@emk-elektronik.de> you wrote:
>
> OK, digging deeper, the culprit actually is:
> 
> 2010-11-28
> 	Mike Frysinger
> 	boot cmds: convert to getenv_yesno() with autostart

There has been agreement to revert that commit.

Best regards,

Wolfgang Denk
Reinhard Meyer Dec. 31, 2010, 11:54 a.m. UTC | #3 
Dear Wolfgang Denk,
> Dear Reinhard Meyer,
>
> In message<4D1DB35A.3090105@emk-elektronik.de>  you wrote:
>>
>> OK, digging deeper, the culprit actually is:
>>
>> 2010-11-28
>> 	Mike Frysinger
>> 	boot cmds: convert to getenv_yesno() with autostart
>
> There has been agreement to revert that commit.
OK, I must have missed that agreement ;)

What about adding a second parameter to getenv_yesno(), that
gives the default value when the env var does not exist?

And maybe move that function into env related source?

Best Regards,
Reinhard
Mike Frysinger Dec. 31, 2010, 5:59 p.m. UTC | #4 
On Friday, December 31, 2010 06:54:17 Reinhard Meyer wrote:
> >> OK, digging deeper, the culprit actually is:
> >> 
> >> 2010-11-28
> >> 
> >> 	Mike Frysinger
> >> 	boot cmds: convert to getenv_yesno() with autostart
> > 
> > There has been agreement to revert that commit.
> 
> OK, I must have missed that agreement ;)
> 
> What about adding a second parameter to getenv_yesno(), that
> gives the default value when the env var does not exist?

that's what i proposed, but Wolfgang would rather revert it seems
-mike
diff mbox

Patch

--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -211,7 +211,7 @@  netboot_common (proto_t proto, cmd_tbl_t *cmdtp, int argc, char * const argv[])
         flush_cache(load_addr, size);

         /* Loading ok, check if we should attempt an auto-start */
-       if (((s = getenv("autostart")) != NULL) && (strcmp(s,"yes") == 0)) {
+       if (getenv_yesno("autostart")) {
                 char *local_args[2];
                 local_args[0] = argv[0];
                 local_args[1] = NULL;