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

Submitted by Simon Glass on Oct. 27, 2011, 12:18 a.m.

Details

Message ID 1319674720-23128-4-git-send-email-sjg@chromium.org
State New, archived
Headers show

Commit Message

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

Comments

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 hide | download patch | download mbox

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