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

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

[Reinhard Meyer]

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?)

Best Regards,
Reinhard

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

[Reinhard Meyer]

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)
--- 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;

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

Reinhard

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

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

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

[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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Spock, did you see the looks on their faces?"
"Yes, Captain, a sort of vacant contentment."

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

[Reinhard Meyer]

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

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

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20101231/d6efaf6e/attachment.pgp 

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

[Stefano Babic]

On 12/31/2010 11:33 AM, 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!

You are right. I am stucking with the same issue, and inverting the test
makes things working again.

However, I see this behavior only with 2010.12 and the suspicious
function is really old, I am wondering why we see this behavior only now.

> 
> (and why is such a generic function in image.c?)

Another good point...

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================