On Sun, Sep 15, 2013 at 1:24 PM, Isaac <[email protected]> wrote: > On Sun, Sep 15, 2013 at 11:59:42AM +0900, Ashwini Sharma wrote: > > On Fri, Sep 13, 2013 at 11:31 PM, Isaac <[email protected]> wrote: > > > I note that it appears to revert one of the recent changes to > > > lib/pending.c. > > > > > > (Would you mind sending new toys as just the *.c file that goes in > toys/*/ > > > when it doesn't need to touch lib/?) > > > my patch on lib/pending.c is only modifying get_int_value() function. > > Checking '-' and leading white spaces in the string. > > > > I changed this > > - if (errno || numstr == ptr || *ptr || rvalue < lowrange || rvalue > > > highrange) > > - perror_exit("bad number '%s'", numstr); > > > > to > > > > + if(errno || *ptr) perror_exit("invalid number '%s'", numstr); > > + if(rvalue < lowrange || rvalue > highrange) > > + error_exit("out of range '%s'", numstr); > > > > for having a better error message. > > > > See this patch in commit 1066 (manually munged to indicate intent...): > > unsigned long rvalue = 0; > char *ptr; > - if(*numstr == '-' || *numstr == '+' || isspace(*numstr)) > perror_exit("invalid number '%s'", numstr); > + > + if (!isdigit(*numstr)) perror_exit("bad number '%s'", numstr); > errno = 0; > rvalue = strtoul(numstr, &ptr, 10); > - if(errno || numstr == ptr) perror_exit("invalid number '%s'", numstr); > - if(*ptr) perror_exit("invalid number '%s'", numstr); > - if(rvalue >= lowrange && rvalue <= highrange) return rvalue; > - else { > - perror_exit("invalid number '%s'", numstr); > - return rvalue; //Not reachable; to avoid waring message. > - } > + if (errno || numstr == ptr || *ptr || rvalue < lowrange || rvalue > > highrange) > + perror_exit("bad number '%s'", numstr); > + return rvalue; > } >
Aah!! when I generated patch, these chages were not reflecting in hg. Better not to apply my lib/pending.c patch > > Rob's mentioned a few times that simpler messages are preferred; > http://landley.net/toybox/cleanup.html refers to this message: > > http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html > > And that's part of the rationale listed in the change. > > (however, I don't see how get_int_value can handle negative numbers; > rvalue and lowrange are unsigned longs...) > > If I were the one doing it, I might use "bad number" and "not in range", > or maybe even for the latter (if it works! Haven't tested) > perror_exit("'%s' too '%s'", numstr, rvalue < lowrange ? "low" : "high"); > But that last might be a little more clever and less clear... > In general, s/invalid/bad/. > > > IPv6 is in fact important. Working on it, but was confused whether to > have > > the IPv6 support > > in the same traceroute.c or separate. As far as I see, there will be lot > > many if/else checks for IPv4/IPv6 cases. > > Like for the incoming packet parsing. > > Same file. A separate IPv6 command is not desireable (Rob recenty > mentioned this somewhere--I think in the discussion of Strake's > mount or sed), though I could see a <name>6 OLDTOY() for those who > are accustomed to the two binaries... > > > Any suggestions are welcome. > > > > regards, > > Ashwini > > Thanks, > Isaac Dunham >
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
