Re: [U-Boot] [PATCH] boot cmds: convert to getenv_yesno() with autostart
Dear Mike Frysinger, In message 201012071657.42065.vap...@gentoo.org you wrote: On Tuesday, December 07, 2010 16:51:41 Wolfgang Denk wrote: Mike Frysinger wrote: hrm, after running this through our test bench, it seems the old code and new code are not functionality equivalent. it boils down to default values when the env var is not set. some places interpret this to mean yes while others expect no. getenv_yesno() takes the no default which doesnt always work. In addition to the default values, there is the difference that the documentation states that autostart has to be set to yes, i. e. a plain y is not supposed to mean 'yes'. consider it an enhancement then ? i dont see a problem with making the values fuzzier. personally, i make funcs like this accept any of the common strings such as true/false/0/1/y/n/yes/no. i can update the API to take a 2nd arg (the default value), or we can punt this patch. it's in next, so there's less pressure to get it fixed immediately ... I think the easiest way to fix this is to revert the commit. it's in next, so it could even just be dropped As it was left unfixed until now I had no other choice but to revert this 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: w...@denx.de The universe does not have laws - it has habits, and habits can be broken. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] boot cmds: convert to getenv_yesno() with autostart
On Tuesday, January 11, 2011 15:04:37 Wolfgang Denk wrote: Mike Frysinger wrote: On Tuesday, December 07, 2010 16:51:41 Wolfgang Denk wrote: Mike Frysinger wrote: hrm, after running this through our test bench, it seems the old code and new code are not functionality equivalent. it boils down to default values when the env var is not set. some places interpret this to mean yes while others expect no. getenv_yesno() takes the no default which doesnt always work. In addition to the default values, there is the difference that the documentation states that autostart has to be set to yes, i. e. a plain y is not supposed to mean 'yes'. consider it an enhancement then ? i dont see a problem with making the values fuzzier. personally, i make funcs like this accept any of the common strings such as true/false/0/1/y/n/yes/no. i can update the API to take a 2nd arg (the default value), or we can punt this patch. it's in next, so there's less pressure to get it fixed immediately ... I think the easiest way to fix this is to revert the commit. it's in next, so it could even just be dropped As it was left unfixed until now I had no other choice but to revert this commit. i explicitly asked about fixing it and you explicitly said you just wanted to revert. so i obviously didnt work on it. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] boot cmds: convert to getenv_yesno() with autostart
Dear Mike Frysinger, In message 201012010634.19596.vap...@gentoo.org you wrote: hrm, after running this through our test bench, it seems the old code and new code are not functionality equivalent. it boils down to default values when the env var is not set. some places interpret this to mean yes while others expect no. getenv_yesno() takes the no default which doesnt always work. In addition to the default values, there is the difference that the documentation states that autostart has to be set to yes, i. e. a plain y is not supposed to mean 'yes'. i can update the API to take a 2nd arg (the default value), or we can punt this patch. it's in next, so there's less pressure to get it fixed immediately ... I think the easiest way to fix this is to revert the commit. Do you agree? 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: w...@denx.de Unix is supported by IBM, like a hanging man is supported by rope. - Don Libes Sandy Ressler: _Life With Unix_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] boot cmds: convert to getenv_yesno() with autostart
On Tuesday, December 07, 2010 16:51:41 Wolfgang Denk wrote: Mike Frysinger wrote: hrm, after running this through our test bench, it seems the old code and new code are not functionality equivalent. it boils down to default values when the env var is not set. some places interpret this to mean yes while others expect no. getenv_yesno() takes the no default which doesnt always work. In addition to the default values, there is the difference that the documentation states that autostart has to be set to yes, i. e. a plain y is not supposed to mean 'yes'. consider it an enhancement then ? i dont see a problem with making the values fuzzier. personally, i make funcs like this accept any of the common strings such as true/false/0/1/y/n/yes/no. i can update the API to take a 2nd arg (the default value), or we can punt this patch. it's in next, so there's less pressure to get it fixed immediately ... I think the easiest way to fix this is to revert the commit. it's in next, so it could even just be dropped -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] boot cmds: convert to getenv_yesno() with autostart
Dear Mike Frysinger, In message 201012071657.42065.vap...@gentoo.org you wrote: In addition to the default values, there is the difference that the documentation states that autostart has to be set to yes, i. e. a plain y is not supposed to mean 'yes'. consider it an enhancement then ? i dont see a problem with making the values fuzzier. personally, i make funcs like this accept any of the common strings such as true/false/0/1/y/n/yes/no. That should be documented, then. And it should be sensible not to accept anything. A typo like yno should IMHO _not_ be interpreted as yes. I think the easiest way to fix this is to revert the commit. it's in next, so it could even just be dropped Yes, but then I have to rebase next... 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: w...@denx.de You shouldn't make my toaster angry. - Household security explained in Johnny Quest ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] boot cmds: convert to getenv_yesno() with autostart
On Tuesday, December 07, 2010 17:19:38 Wolfgang Denk wrote: Mike Frysinger wrote: In addition to the default values, there is the difference that the documentation states that autostart has to be set to yes, i. e. a plain y is not supposed to mean 'yes'. consider it an enhancement then ? i dont see a problem with making the values fuzzier. personally, i make funcs like this accept any of the common strings such as true/false/0/1/y/n/yes/no. That should be documented, then. And it should be sensible not to accept anything. A typo like yno should IMHO _not_ be interpreted as yes. good point ... i hadnt thought of that I think the easiest way to fix this is to revert the commit. it's in next, so it could even just be dropped Yes, but then I have to rebase next... imo, keeping the next branch clean at the cost of unstableness isnt a problem. this seems to be the convention that Linux started and other people follow. personally, when i use next from anywhere, the first thing i do is reset my local next branch to whatever the current upstream state. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] boot cmds: convert to getenv_yesno() with autostart
On Sunday, November 28, 2010 16:02:49 Wolfgang Denk wrote: Mike Frysinger wrote: Use the new helper func to clean up duplicate logic handling of the autostart env var. Signed-off-by: Mike Frysinger vap...@gentoo.org --- common/cmd_fdc.c |3 +-- common/cmd_fdos.c |2 +- common/cmd_ide.c |2 +- common/cmd_nand.c |4 ++-- common/cmd_net.c |2 +- common/cmd_scsi.c |2 +- common/cmd_usb.c |2 +- 7 files changed, 8 insertions(+), 9 deletions(-) Applied to next branch, thanks. hrm, after running this through our test bench, it seems the old code and new code are not functionality equivalent. it boils down to default values when the env var is not set. some places interpret this to mean yes while others expect no. getenv_yesno() takes the no default which doesnt always work. i can update the API to take a 2nd arg (the default value), or we can punt this patch. it's in next, so there's less pressure to get it fixed immediately ... -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] boot cmds: convert to getenv_yesno() with autostart
Dear Mike Frysinger, In message 1287573475-6737-1-git-send-email-vap...@gentoo.org you wrote: Use the new helper func to clean up duplicate logic handling of the autostart env var. Signed-off-by: Mike Frysinger vap...@gentoo.org --- common/cmd_fdc.c |3 +-- common/cmd_fdos.c |2 +- common/cmd_ide.c |2 +- common/cmd_nand.c |4 ++-- common/cmd_net.c |2 +- common/cmd_scsi.c |2 +- common/cmd_usb.c |2 +- 7 files changed, 8 insertions(+), 9 deletions(-) Applied to next branch, thanks. 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: w...@denx.de Work 8 hours, sleep 8 hours; but not the same 8 hours. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
