Patchwork [U-Boot,3/3] net: Be less picky about decoding the netretry env var

login
register
mail settings
Submitter Simon Glass
Date Oct. 27, 2011, 12:18 a.m.
Message ID <1319674720-23128-4-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/122046/
State New, archived
Headers show

Comments

Simon Glass - Oct. 27, 2011, 12:18 a.m.
This is intended purely as a code size reduction.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 net/net.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Mike Frysinger - Oct. 27, 2011, 6 a.m.
On Thu, Oct 27, 2011 at 02:18, Simon Glass wrote:
> --- a/net/net.c
> +++ b/net/net.c
>
> -               if (!strcmp(nretry, "yes"))
> +               if (*nretry == 'y')

not sure about this as it makes it hard to add code in the future if
we care about compatibility.  if we support just "y", people will
start using it.
-mike
Simon Glass - Oct. 27, 2011, 1:35 p.m.
Hi Mike,

On Wed, Oct 26, 2011 at 11:00 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thu, Oct 27, 2011 at 02:18, Simon Glass wrote:
>> --- a/net/net.c
>> +++ b/net/net.c
>>
>> -               if (!strcmp(nretry, "yes"))
>> +               if (*nretry == 'y')
>
> not sure about this as it makes it hard to add code in the future if
> we care about compatibility.  if we support just "y", people will
> start using it.
> -mike
>

Yes, I'm not sure either. I have found a few env variables that use
single letters (autoload, flashchecksum) and noticed that quite a few
commands only decode as much of their subcommand as they need to be
unique. I am happier with y and n than o!

Regards,
Simon
Mike Frysinger - Oct. 27, 2011, 1:40 p.m.
On Thu, Oct 27, 2011 at 15:35, Simon Glass wrote:
> On Wed, Oct 26, 2011 at 11:00 PM, Mike Frysinger wrote:
>> On Thu, Oct 27, 2011 at 02:18, Simon Glass wrote:
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>>
>>> -               if (!strcmp(nretry, "yes"))
>>> +               if (*nretry == 'y')
>>
>> not sure about this as it makes it hard to add code in the future if
>> we care about compatibility.  if we support just "y", people will
>> start using it.
>
> Yes, I'm not sure either. I have found a few env variables that use
> single letters (autoload, flashchecksum) and noticed that quite a few
> commands only decode as much of their subcommand as they need to be
> unique. I am happier with y and n than o!

right, u-boot is inconsistent

the short command names are a bit unique in that we already say "if
you don't use the full command name, that's your fault".  after all,
how much of the short name you need to use changes based on the
commands enabled and the autocomplete define being enabled.

if you enable "reginfo" and "reset", then you can't use "r" or "re".
but if you only have "reset", then you can use "r" and "re" to reset.

env vars are slightly different in that they're not terribly well
documented.  so people look at the source to see what is accepted and
then do that.  maybe the answer is to document the exact values and
then when people screw up, we can point to that as a fallback.
-mike
Wolfgang Denk - Oct. 27, 2011, 9:02 p.m.
Dear Simon Glass,

In message <CAPnjgZ1RDqAER6aWZ08o7pymW98o8-1y8pMM2p6NNv2XWn44LA@mail.gmail.com> you wrote:
>
> Yes, I'm not sure either. I have found a few env variables that use
> single letters (autoload, flashchecksum) and noticed that quite a few
> commands only decode as much of their subcommand as they need to be
> unique. I am happier with y and n than o!

The 'o' indeed kills this.  I don't think this is a good idea.

Best regards,

Wolfgang Denk

Patch

diff --git a/net/net.c b/net/net.c
index cd34bf9..3712e17 100644
--- a/net/net.c
+++ b/net/net.c
@@ -605,11 +605,11 @@  void NetStartAgain(void)
 
 	nretry = getenv("netretry");
 	if (nretry) {
-		if (!strcmp(nretry, "yes"))
+		if (*nretry == 'y')
 			retry_forever = 1;
-		else if (!strcmp(nretry, "no"))
+		else if (!*nretry == 'n')
 			retrycnt = 0;
-		else if (!strcmp(nretry, "once"))
+		else if (*nretry == 'o')	/* "once" */
 			retrycnt = 1;
 		else
 			retrycnt = simple_strtoul(nretry, NULL, 0);