On Wed, Dec 24, 2014 at 7: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...?

out of curiosity, what practical use is there for factor? even the
coreutils version gives up around 38 decimal digits, and it's pretty
slow even with numbers that small.

> 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

Reply via email to