Dear Robin Getz,
In message 200907171745.36176.rgetz@blackfin.uclinux.org you wrote:
You probably should add a doc/README.* file to explain how that is supposed to be used.
Will do.
What is the rule for when things go in ./doc/README.* vs ./README ?
Size - anything that takes more than 5...10 lines ?
Could you generate a git patch instead?
Sure - I'll generate something from the net tree?
Please use the mainline repo / "master" branch as reference.
+#if defined(CONFIG_CMD_DNS) +extern char NetDNSResolve[255]; /* The host to resolve */ +#endif
Can this buffer overflow?
Shouldn't.
Agreed. I've seen the tests later in the code but forgot to remove this.
...
Hmmm... why do you assume that the address we're trying to resolve is the server IP?
To me it makes at least as much sense to resolve the "ipaddr" value.
Yeah, this was my biggest issue for things too (from the original patch).
but you can easily :
set nameip ${serverip}
to transfer it to something else. - or just use it directly.
But you lost the original "serverip" setting, which may not be wanted.
- *p++ = 0; /* Mark end of host name */
- *p++ = 0; /* Well, lets put this byte as well */
Why that?
No idea - that was in the original....
Hmm -- according to my TCP/IP Illustrated - names are suppost to be double null terminated - so I think that is a requirement.
Then please remove the comment that automatically triggers such "why that?" questions (and eventually replace it with a better explanation).
I suggest to change this as follows:
...
What do you think?
that is more flexible that the current implementation...
Indeed :-)
but also a little more complex.
Not really. Yes, you have to add 3 lines of code to check for the 2nd argument, butexcept from that you just change the
setenv("serverip", IPStr); into setenv(argvp, IPStr);
saving it in a predefined env var (like 'dnshostip') is also OK.
dns www.denx.de set serverip ${dnshostip}
-- is going to be a little smaller...
Up to you...
I like my proposal better, as it avoids adding another predefined env var which is of not much use to most users, as it will only serve as temp storage; and your proposal requires a second command, i. e. more typing and thus more chances for typos.
Best regards,
Wolfgang Denk