On 11/22/2013 04:20:07 AM, Ashwini Sharma wrote:
Hi Rob, list,

I knwo its too many mails in one day, but thats due to malfunctioned
mail I had.

No problem. I'm working to catch up. (My multi-week cold has mosty cleared up, except for the part that seems to have turned into a mild but lingering case of bronchitis.)

Attacehd is the patch for ifconfig.c,

I actually restarted ifconfig cleanup shortly before you sent this, and don't know what parts of this still applies. Let's see...

fixing issues

1. show_help(), only prints help and continues the execution.
    ifconfig had a SEGFAULT due to this very thing. Modified the
    the ifconfig_main() to return after showing help for ifconfig.

That still applies. It would be nice if it printed _which_ command needed an argument though. (This looks like a job for error_exit().)

I have lib/args.c saying "-c needs 1 argument" so "needs argument" isn't new vocabulary to expect people to learn. So I can say "netmask needs argument". (I could also say "netmask...?" but I'm not sure that's easier for non-english speakers to understand. Hmmm.)

The next question is, if I have a specific message does dumping the help text improve matters, or does it just make the error message more likely to get lost? Especially since at the moment if I go "ifconfig walrus" it just says "ifconfig: walrus: no such device" without dumping help text...

The crazy part is that the upstream version behaves both ways, "ifconfig blah" doesn't give help text, and "ifconfig netmask" does. I suppose the useful difference is:

  $ ifconfig -x
  ifconfig: option `-x' not recognised.
  ifconfig: `--help' gives usage information.
  $ ./ifconfig -x
  ifconfig: -x: No such device

I.E. the toybox version doesn't hint about "--help". Does the fact that most toybox commands support --help make up for this? Not sure...

Maybe I should add a new optstring character to exit with help text for unknown arguments?

For the moment I'm yanking the show_help(), having error_exit() say what the actual problem was, and if we want lib/args.c to say "try --help" for an unknown command line option, it should probably do it globally.

(I'm open to suggestions on this. I'm a bit under the weather still and not really feeling up to making aesthetic design decisions right now. Functional, sure, but aesthetic... not so much.)

In the meantime, erring on the side of being terse, because it's unix _and_ embedded.

2. In __ display_ifconfig() _ there was typo, corrected the filename to
    "/proc/net/if_inet6".

I caught that one independently.

3. in __ show_iface() _ data was read into toybuf. after this _name_
    pointed inside the toybuf.  This function passed name to __
display_ifconfig() _
    function, which again used the toybuf to read data into. Hence
corrupting the
_name_. Fixed this by making a copy of name and then freeing it after use.

I think I fixed this already? (I found a couple of these while doing the ipv6 cleanup, changed it so display_ifconfig either got iface_name or the copy in sl, and neither lives in toybuf.)

Thanks,

Rob
_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Reply via email to