Yes, I apparently submitted a diff from the tree I used to find the problem, not the tree I cleanly patched... (Oh well, I'm really stale at C, so it wasn't *that* clean)
I literally discovered that problem by accident, and submitted a patch because (bad) code was quicker than a good explanation. On Dec 24, 2014 9:47 PM, "Rob Landley" <[email protected]> wrote: > On 12/12/14 17:19, Robert Thompson wrote: > > I ran across a variance between toybox factor and coreutils factor. > > > > Coreutils factor will accept numbers on stdin separated by any whitespace > > (including newlines and tabs) between integers, but toybox factor was > only > > accepting one integer per line. > > Really? > > $ factor "" > factor: `' is not a valid positive integer > $ factor "32 " > factor: `32 ' is not a valid positive integer > $ factor "32 7" > factor: `32 7' is not a valid positive integer > > Must be newer than Ubuntu 12.04... Ah, on _stdin_. Right. Confirmed. > > Hmmm... might as well make it take both anyway. > > > I added a test for this, and hacked factor to give the expected behavior. > > It's not properly indented, and it depends on isspace(), but it seems to > be > > doing the job. > > I think you left a debug printf in there, it's making all the tests fail, > including the ones you submitted: > > $ VERBOSE=fail scripts/test.sh factor > scripts/make.sh > Generate headers from toys/*/*.c... > Make generated/config.h from .singleconfig. > generated/flags.h generated/help.h > Compile toybox..... > FAIL: factor -32 > echo -ne '' | factor -32 > --- expected 2014-12-23 20:48:38.689595406 -0600 > +++ actual 2014-12-23 20:48:38.693595406 -0600 > @@ -1 +1,2 @@ > +->: -32 > -32: -1 2 2 2 2 2 > > A couple other issues: > > @@ -20,9 +20,11 @@ > static void factor(char *s) > { > long l, ll; > + while( *s && s[0] && ! isspace(s[0]) ) { > + printf("->: %s\n",s); > > l = strtol(s, &s, 0); > > *s and s[0] are the same thing. > > @@ -61,6 +63,7 @@ > } > } > xputc('\n'); > + } > } > > void factor_main(void) > > As you mentioned, you added a curly bracket level without indenting the > code. > I could do a tail call and expect the compiler to turn the recursion into > iteration, but reindenting the code properly is worth the noise in the > diff. > > The version I checked in won't error out for 'factor ""' or 'factor "36 "' > the way Ubuntu's will, but I think I'm ok with that...? > > Let me know if there are more things to fix. > > Thanks, > > Rob >
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
