tftp "command" in default environments
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
788 4853 59575
$ git grep -w tftp include/configs/ board/ | wc
658 4095 51357
$ git grep -w tftp doc/ README | wc
119 685 7661This 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" <[email protected]>
Comelit Group S.p.a.
Quoted 10 lines · Luca Ceresoli
Hi,
since
7a83af07aef3c(TFTP: add tftpsrv command) U-Boot has a new "tftpsrv" command, which can be enabled by definingCONFIG_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?
Quoted 10 lines
A few numbers (consider a little percentage of false positives):
$ git grep -w tftp | wc 788 4853 59575 $ git grep -w tftp include/configs/ board/ | wc 658 4095 51357 $ git grep -w tftp doc/ README | wc 119 685 7661This is of course a bug in many boards (and the docs too).
Quoted 7 lines
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
Quoted 3 lines
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
Quoted 2 lines
OTOH it would give no visible advantage to those boards that will never want tftpsrv at all, which are probably the vast majority.
yep.
Signature · 2 lines
Regards, Igor.
Quoted 5 lines · 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").
Quoted 1 line
This is of course a bug in many boards (and the docs too).
submit patches
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110622/dd5de1b9/attachment.pgp
Dear Mike Frysinger,
Quoted 8 lines · In message <[email protected]> you wrote:
Quoted 6 lines
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.
Quoted 2 lines
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
Signature · 4 lines
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 at denx.de "A dirty mind is a joy forever." - Randy Kunkee
Quoted 7 lines · On Wednesday, June 22, 2011 13:26:36 Wolfgang Denk wrote:
Quoted 6 lines · Mike Frysinger
Quoted 5 lines
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.
Quoted 6 lines
Quoted 2 lines
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110622/90f2f2b8/attachment.pgp
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?
Signature · 1 line
Eric Cooper e c c @ c m u . e d u
Dear "Luca Ceresoli",
Quoted 3 lines · In message <[email protected]> you wrote:
since
7a83af07aef3c(TFTP: add tftpsrv command) U-Boot has a new "tftpsrv" command, which can be enabled by definingCONFIG_CMD_TFTPSRV.
Right.
Quoted 1 line
Unfortunately, activating it causes some boards to stop working correctly.
I think this is not quite correct. I don't see any broken board.
Quoted 4 lines
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.
Quoted 1 line
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.
Quoted 2 lines
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.
Quoted 2 lines
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
Signature · 4 lines
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 at denx.de In C we had to code our own bugs, in C++ we can inherit them.