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

newer
[U-Boot] Work Smarter

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

older
[U-Boot] [PATCH v3 7/8] imximage:...

Reinhard Meyer

31 Dec 2010 31 Dec '10

11:33 a.m.

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

Reply

Show replies by date

Reinhard Meyer

31 Dec 31 Dec

11:41 a.m.

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

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

Reply

Stefano Babic

11:58 a.m.

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

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@denx.de =====================================================================

Reply

Wolfgang Denk

12:42 p.m.

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@denx.de "Spock, did you see the looks on their faces?" "Yes, Captain, a sort of vacant contentment."

Reply

Reinhard Meyer

12:54 p.m.

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

Dear Wolfgang Denk,

Dear Reinhard Meyer,

In message4D1DB35A.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

Reply

Mike Frysinger

6:59 p.m.

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

Reply

Stefano Babic

11:55 a.m.

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

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@denx.de =====================================================================

Reply

5245

Age (days ago)

5245

Last active (days ago)

Download

6 comments

4 participants

tags (0)

participants (4)

Mike Frysinger
Reinhard Meyer
Stefano Babic
Wolfgang Denk