[U-Boot] tftp command in default environments

2011-06-22 Thread Luca Ceresoli
Hi,

since 7a83af07aef3c (TFTP: add tftpsrv command) U-Boot has a new tftpsrv
command, which can be enabled by defining CONFIG_CMD_TFTPSRV.

Unfortunately, activating it causes some boards to stop working correctly.
This is because they use the tftp command in their environment, but tftp is
has never been a real command, just the prefix for tftpboot.
tftpboot used to be the sole completion for the tftp prefix, and this is still
true unless one activates tftpsrv.

A few numbers (consider a little percentage of false positives):
$ git grep -w tftp | wc
 7884853   59575
$ git grep -w tftp include/configs/ board/ | wc
 6584095   51357
$ git grep -w tftp doc/ README | wc
 119 6857661

This is of course a bug in many boards (and the docs too).

It's obviously to be fixed for boards that define CONFIG_CMD_TFTPSRV:
currently none, but soon I'll update dig297 in this sense.

Fixing the docs would be wise, too. I'll try to find some time to do this job.

What I'm not sure about is if the default environment for all boards should be
fixed as well.
This would increase the binary size and the environment size. The increase
would be of a few bytes, is it correct to assume that it is negligible for all
boards?
OTOH it would give no visible advantage to those boards that will never want
tftpsrv at all, which are probably the vast majority.

I look forward to receive comments on this last question.

Luca



 
Luca Ceresoli
 

Luca Ceresoli luca.ceres...@comelit.it
Comelit Group S.p.a.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] tftp command in default environments

2011-06-22 Thread Igor Grinberg
On 06/22/11 17:28, Luca Ceresoli wrote:

 Hi,

 since 7a83af07aef3c (TFTP: add tftpsrv command) U-Boot has a new tftpsrv
 command, which can be enabled by defining CONFIG_CMD_TFTPSRV.

 Unfortunately, activating it causes some boards to stop working correctly.
 This is because they use the tftp command in their environment, but tftp is
 has never been a real command, just the prefix for tftpboot.
 tftpboot used to be the sole completion for the tftp prefix, and this is still
 true unless one activates tftpsrv.

Don't enable the CONFIG_CMD_TFTPSRV by default.
Only boards that want to use the tftpsrv command,
should define the above config.
This makes it like this: if you define the CONFIG_CMD_TFTPSRV,
then you need to make sure your default environment is still good,
if you don't define it then you should not have a problem.
Right?

 A few numbers (consider a little percentage of false positives):
 $ git grep -w tftp | wc
  7884853   59575
 $ git grep -w tftp include/configs/ board/ | wc
  6584095   51357
 $ git grep -w tftp doc/ README | wc
  119 6857661

 This is of course a bug in many boards (and the docs too).

 It's obviously to be fixed for boards that define CONFIG_CMD_TFTPSRV:
 currently none, but soon I'll update dig297 in this sense.

 Fixing the docs would be wise, too. I'll try to find some time to do this job.

 What I'm not sure about is if the default environment for all boards should be
 fixed as well.

See above

 This would increase the binary size and the environment size. The increase
 would be of a few bytes, is it correct to assume that it is negligible for all
 boards?

looks like a non issue

 OTOH it would give no visible advantage to those boards that will never want
 tftpsrv at all, which are probably the vast majority.

yep.



-- 
Regards,
Igor.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] tftp command in default environments

2011-06-22 Thread Mike Frysinger
On Wednesday, June 22, 2011 10:28:59 Luca Ceresoli wrote:
 Unfortunately, activating it causes some boards to stop working correctly.
 This is because they use the tftp command in their environment, but tftp
 is has never been a real command, just the prefix for tftpboot.
 tftpboot used to be the sole completion for the tftp prefix, and this is
 still true unless one activates tftpsrv.

those configs are broken.  the autocomplete support has never been something 
you can rely on if you add new commands.  if you want correct behavior all the 
time, then use the full proper name (i.e. tftpboot not tftp).

 This is of course a bug in many boards (and the docs too).

submit patches
-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] tftp command in default environments

2011-06-22 Thread Wolfgang Denk
Dear Mike Frysinger,

In message 201106221207.42499.vap...@gentoo.org you wrote:

  Unfortunately, activating it causes some boards to stop working correctly.
  This is because they use the tftp command in their environment, but tftp
  is has never been a real command, just the prefix for tftpboot.
  tftpboot used to be the sole completion for the tftp prefix, and this is
  still true unless one activates tftpsrv.

 those configs are broken.  the autocomplete support has never been something 

This has absolutely nothing to do with autocomplete.  It's just using
the fact that you can abbreviate all command names as long as they
remain unambiguous.

 you can rely on if you add new commands.  if you want correct behavior all 
 the 
 time, then use the full proper name (i.e. tftpboot not tftp).

True.

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
A dirty mind is a joy forever.   - Randy Kunkee
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] tftp command in default environments

2011-06-22 Thread Wolfgang Denk
Dear Luca Ceresoli,

In message 4e01fc2b.3010...@comelit.it you wrote:
 
 since 7a83af07aef3c (TFTP: add tftpsrv command) U-Boot has a new tftpsrv
 command, which can be enabled by defining CONFIG_CMD_TFTPSRV.

Right.

 Unfortunately, activating it causes some boards to stop working correctly.

I think this is not quite correct.  I don't see any broken board.

 This is because they use the tftp command in their environment, but tftp is
 has never been a real command, just the prefix for tftpboot.
 tftpboot used to be the sole completion for the tftp prefix, and this is still
 true unless one activates tftpsrv.

Correct.  So only those board maintainers that use tftp as
abbreviation _and_ now changing their board configuration to include
the tftpsrv need to pay attention.

 This is of course a bug in many boards (and the docs too).

It is not a bug.  It is a pretty normal situation that people use
abbreviations as long as they work.  When working interactively, I
used to use set for a long, long time instead of setenv, until it
stopped working.  Then I used to use sete (and still do this) except
for boards that enable for example the setexpr command.

We just should try not to use such abbreviations in the environment,
and/or pay special attention to check the environment when changing
the board configuration to include new, additional commands.

 It's obviously to be fixed for boards that define CONFIG_CMD_TFTPSRV:
 currently none, but soon I'll update dig297 in this sense.

Then make sure to check the default env.

 What I'm not sure about is if the default environment for all boards should be
 fixed as well.

No.  Most of them will never enable the tftpsrv command.


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
In C we had to code our own bugs, in C++ we can inherit them.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] tftp command in default environments

2011-06-22 Thread Mike Frysinger
On Wednesday, June 22, 2011 13:26:36 Wolfgang Denk wrote:
 Mike Frysinger wrote:
   Unfortunately, activating it causes some boards to stop working
   correctly. This is because they use the tftp command in their
   environment, but tftp is has never been a real command, just the
   prefix for tftpboot. tftpboot used to be the sole completion for the
   tftp prefix, and this is still true unless one activates tftpsrv.
  
  those configs are broken.  the autocomplete support has never been
  something
 
 This has absolutely nothing to do with autocomplete.  It's just using
 the fact that you can abbreviate all command names as long as they
 remain unambiguous.

that's what i was referring to
-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] tftp command in default environments

2011-06-22 Thread Eric Cooper
Since tftpsrv is new, wouldn't it make sense to change it to
srvtftp or something else that doesn't conflict with a common
abbreviation?

-- 
Eric Cooper e c c @ c m u . e d u
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot