[Toybox] [Patch] - ifconfig, fixes for IPv6 and SEGFAULT
Rob Landley
rob at landley.net
Sun Dec 22 18:14:36 PST 2013
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
More information about the Toybox
mailing list