Re: [Toybox] [PATCH] Fix various seq bugs.

2017-09-10 Thread Rob Landley 


On 09/10/2017 02:00 PM, enh wrote:
> On Fri, Sep 8, 2017 at 4:10 PM, Rob Landley  wrote:
>> On 05/23/2017 02:18 AM, Josh Gao wrote:
>>> On Mon, May 22, 2017 at 11:54 PM, Rob Landley >> > wrote:
>>>
>>> What's the use case for this code? Did they notice a difference from gnu
>>> and say "any difference is a bug", or was somebody actually trying to do
>>> something that broke?
>>>
>>>
>>> The surprising behavior that I ran into was this:
>>>
>>> $ seq 100 101
>>> 1e+06
>>> 1e+06
>>
>> Ok, digging back down to this, that was the only issue you hit? It
>> should _not_ spontaneously produce engineering notation output? (Agreed,
>> of course...)
> 
> (sorry, been sick.)

I sat on the issue for 4 months, so not your fault.

> yes, the only bug that was reported was that.
> 
> all the rest were based on poking at GNU seq to see how it behaves. as
> yet i've had no requests for any of the oddities i found.

I wound up keeping the "increment 0 means no output" and "last sets
precision too" differences from the seq in ubuntu. They're easy to
change but I'd like a reason other than "it's different"...

(Those seq behaviors aren't _internally_ self-consistent: why would the
first 2 arguments set precision but not the third? Why would you produce
no output for last < first but endless output for last == first? A
standard for this command would be so nice...)

Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Fix various seq bugs.

2017-09-10 Thread enh 
On Fri, Sep 8, 2017 at 4:10 PM, Rob Landley  wrote:
> On 05/23/2017 02:18 AM, Josh Gao wrote:
>> On Mon, May 22, 2017 at 11:54 PM, Rob Landley > > wrote:
>>
>> What's the use case for this code? Did they notice a difference from gnu
>> and say "any difference is a bug", or was somebody actually trying to do
>> something that broke?
>>
>>
>> The surprising behavior that I ran into was this:
>>
>> $ seq 100 101
>> 1e+06
>> 1e+06
>
> Ok, digging back down to this, that was the only issue you hit? It
> should _not_ spontaneously produce engineering notation output? (Agreed,
> of course...)

(sorry, been sick.)

yes, the only bug that was reported was that.

all the rest were based on poking at GNU seq to see how it behaves. as
yet i've had no requests for any of the oddities i found.

> Because you did other stuff while you were there:
>
> 1) You changed tests/seq.test to make sure we reproduce the bug where
> seq acts like yes, counting by zero. Is there a use case for this bug?
> (Yes exists, output of seq was previously guaranteed to terminate... I
> see busybox reproduces the ubuntu behavior but that's their default
> position in the absence of a spec, I'm trying to figure out what's the
> right thing to do...)
>
> 2) You have code to checking and make sure "1.234e06" maintains the same
> number of digits of precision. Is there something that cares about that?
> Your test is 1.0e0, which is the same with and without the e0?
>
> (I sometimes use added tests as guidance for "use cases you care about",
> and I'm not really getting it from these...)
>
> Rob
>
> P.S. We got GPS to work at $DAYJOB! Yay! Still lots to do but light at
> the end of the darn tunnel...
> ___
> Toybox mailing list
> Toybox@lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Fix various seq bugs.

2017-09-09 Thread Rob Landley 
On 09/08/2017 06:10 PM, Rob Landley wrote:
> On 05/23/2017 02:18 AM, Josh Gao wrote:
>> On Mon, May 22, 2017 at 11:54 PM, Rob Landley > > wrote:
>>
>> What's the use case for this code? Did they notice a difference from gnu
>> and say "any difference is a bug", or was somebody actually trying to do
>> something that broke?
>>
>>
>> The surprising behavior that I ran into was this:
>>
>> $ seq 100 101
>> 1e+06
>> 1e+06
> 
> Ok, digging back down to this, that was the only issue you hit? It
> should _not_ spontaneously produce engineering notation output? (Agreed,
> of course...)
> 
> Because you did other stuff while you were there:
> 
> 1) You changed tests/seq.test to make sure we reproduce the bug where
> seq acts like yes, counting by zero. Is there a use case for this bug?
> (Yes exists, output of seq was previously guaranteed to terminate... I
> see busybox reproduces the ubuntu behavior but that's their default
> position in the absence of a spec, I'm trying to figure out what's the
> right thing to do...)
> 
> 2) You have code to checking and make sure "1.234e06" maintains the same
> number of digits of precision. Is there something that cares about that?
> Your test is 1.0e0, which is the same with and without the e0?
> 
> (I sometimes use added tests as guidance for "use cases you care about",
> and I'm not really getting it from these...)

Also, this test:

  FAIL: seq precision last
  echo -ne '' | seq -s, 1.0 2.0 4.00
  --- expected  2017-09-09 19:11:23.901639181 -0500
  +++ actual2017-09-09 19:11:23.909639181 -0500
  @@ -1 +1 @@
  -1.0,3.0
  +1.00,3.00

Why do the first two affect the requested precision but the third doesn't?

Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Fix various seq bugs.

2017-09-08 Thread Rob Landley 
On 05/23/2017 02:18 AM, Josh Gao wrote:
> On Mon, May 22, 2017 at 11:54 PM, Rob Landley  > wrote:
> 
> What's the use case for this code? Did they notice a difference from gnu
> and say "any difference is a bug", or was somebody actually trying to do
> something that broke?
> 
> 
> The surprising behavior that I ran into was this:
> 
> $ seq 100 101
> 1e+06
> 1e+06

Ok, digging back down to this, that was the only issue you hit? It
should _not_ spontaneously produce engineering notation output? (Agreed,
of course...)

Because you did other stuff while you were there:

1) You changed tests/seq.test to make sure we reproduce the bug where
seq acts like yes, counting by zero. Is there a use case for this bug?
(Yes exists, output of seq was previously guaranteed to terminate... I
see busybox reproduces the ubuntu behavior but that's their default
position in the absence of a spec, I'm trying to figure out what's the
right thing to do...)

2) You have code to checking and make sure "1.234e06" maintains the same
number of digits of precision. Is there something that cares about that?
Your test is 1.0e0, which is the same with and without the e0?

(I sometimes use added tests as guidance for "use cases you care about",
and I'm not really getting it from these...)

Rob

P.S. We got GPS to work at $DAYJOB! Yay! Still lots to do but light at
the end of the darn tunnel...
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Fix various seq bugs.

2017-05-23 Thread Josh Gao 
On Mon, May 22, 2017 at 11:54 PM, Rob Landley  wrote:
>
> What's the use case for this code? Did they notice a difference from gnu
> and say "any difference is a bug", or was somebody actually trying to do
> something that broke?
>

The surprising behavior that I ran into was this:

$ seq 100 101
1e+06
1e+06
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Fix various seq bugs.

2017-05-22 Thread Rob Landley 
On 05/22/2017 12:49 PM, enh wrote:
> ping?

Sigh. I had a message half-composed when my netbook crashed last week,
noting things like how this:

> Match GNU/busybox behavior with 0 increment. (An existing test was
> failing on the host because of this.)

Is unfixing a bug I fixed. I explicitly made it not do that because if
you want "yes" it exists, and a script had a hang due to accidental
infinite output from seq with a zero increment.

Let's see, what's this precision thing...

  $ seq 1.234e3  | tail -n 3
  9997.000
  9998.000
  .000

That's nice. You want to reproduce that exactly? Yes, there's code for it...

This is another patch adding a 20 line function to do something dubious.
I added -f way back when so you could micromanage this if you cared,
there's no posix spec on it, and the man page doesn't mention this, I
pulled up the old Red Hat 9 image I left on
https://busybox.net/downloads/qemu/ way back in the dawn of time, and:

  $ seq 1 2.000 6
  1
  3
  5

So this precision padding business was not an original feature, and got
added at some point. (Knoppix 6.7 from 2011 has it so it's not exactly
_recent_, but still.)

Did this actually confuse somebody?

 to 1. Two arguments are used as first and last. Arguments can be
-negative or floating point.
+positive/negative and integer/floating point.

The arguments _aren't_ integer, doubles have 53 bits of precision. Feed
it a 64 bit number it's not going to work, the help text might as well
say it's a double...

What's the use case for this code? Did they notice a difference from gnu
and say "any difference is a bug", or was somebody actually trying to do
something that broke?

Sigh, lemme see what I can do with this patch...

Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Fix various seq bugs.

2017-05-22 Thread enh 
ping?

On Sat, May 13, 2017 at 12:49 PM, enh  wrote:
> i wasn't seriously considering implementing the arbitrary-precision
> arithmetic that GNU seems to use, but i did wonder about at least
> using long long instead of double for 53 deciding i was already far enough down the rat hole...
>
> (on Android, where we already link against boringssl for the assembler
> hash functions, we could easily use the assembler bignum functions
> too, but both of us have better things to do!)
>
> the specific bug report was bad behavior around 100 because of %g.
> increasing that to 53 bits should be enough for anyone... :-)
>
> On Sat, May 13, 2017 at 12:36 PM, enh  wrote:
>> Use the precision of the inputs to determine the default output. This was the
>> reported bug. (Two existing tests were failing on the host because of this,
>> so I've fixed those tests and added more tests for other special cases.)
>>
>> Reject invalid inputs.
>>
>> Match GNU/busybox behavior with 0 increment. (An existing test was failing on
>> the host because of this.)
>>
>> Also remove a couple of locals that were duplicating globals.
>>
>> Bug: http://b/37792952
>> ---
>>  tests/seq.test | 23 +++---
>>  toys/lsb/seq.c | 74 
>> ++
>>  2 files changed, 69 insertions(+), 28 deletions(-)
>
>
>
> --
> Elliott Hughes - http://who/enh - http://jessies.org/~enh/
> Android native code/tools questions? Mail me/drop by/add me as a reviewer.



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Fix various seq bugs.

2017-05-13 Thread enh 
i wasn't seriously considering implementing the arbitrary-precision
arithmetic that GNU seems to use, but i did wonder about at least
using long long instead of double for 53 wrote:
> Use the precision of the inputs to determine the default output. This was the
> reported bug. (Two existing tests were failing on the host because of this,
> so I've fixed those tests and added more tests for other special cases.)
>
> Reject invalid inputs.
>
> Match GNU/busybox behavior with 0 increment. (An existing test was failing on
> the host because of this.)
>
> Also remove a couple of locals that were duplicating globals.
>
> Bug: http://b/37792952
> ---
>  tests/seq.test | 23 +++---
>  toys/lsb/seq.c | 74 
> ++
>  2 files changed, 69 insertions(+), 28 deletions(-)



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

[Toybox] [PATCH] Fix various seq bugs.

2017-05-13 Thread enh 
Use the precision of the inputs to determine the default output. This was the
reported bug. (Two existing tests were failing on the host because of this,
so I've fixed those tests and added more tests for other special cases.)

Reject invalid inputs.

Match GNU/busybox behavior with 0 increment. (An existing test was failing on
the host because of this.)

Also remove a couple of locals that were duplicating globals.

Bug: http://b/37792952
---
 tests/seq.test | 23 +++---
 toys/lsb/seq.c | 74 ++
 2 files changed, 69 insertions(+), 28 deletions(-)
From d1112d0eb4d45b3f2e592e058a433fbe2484e63f Mon Sep 17 00:00:00 2001
From: Elliott Hughes 
Date: Sat, 13 May 2017 12:29:24 -0700
Subject: [PATCH] Fix various seq bugs.

Use the precision of the inputs to determine the default output. This was the
reported bug. (Two existing tests were failing on the host because of this,
so I've fixed those tests and added more tests for other special cases.)

Reject invalid inputs.

Match GNU/busybox behavior with 0 increment. (An existing test was failing on
the host because of this.)

Also remove a couple of locals that were duplicating globals.

Bug: http://b/37792952
---
 tests/seq.test | 23 +++---
 toys/lsb/seq.c | 74 ++
 2 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/tests/seq.test b/tests/seq.test
index 7107978..a7b8068 100755
--- a/tests/seq.test
+++ b/tests/seq.test
@@ -19,9 +19,11 @@ testing "count up by 2" "seq 4 2 8" "4\n6\n8\n" "" ""
 testing "count down by 2" "seq 8 -2 4" "8\n6\n4\n" "" ""
 testing "count wrong way #1" "seq 4 -2 8" "" "" ""
 testing "count wrong way #2" "seq 8 2 4" "" "" ""
-testing "count by .3" "seq 3 .3 4" "3\n3.3\n3.6\n3.9\n" "" ""
-testing "count by -.9" "seq .7 -.9 -2.2" "0.7\n-0.2\n-1.1\n-2\n" "" ""
-testing "count by zero" "seq 4 0 8 | head -n 10" "" "" ""
+testing "count by .3" "seq 3 .3 4" "3.0\n3.3\n3.6\n3.9\n" "" ""
+testing "count by -.9" "seq .7 -.9 -2.2" "0.7\n-0.2\n-1.1\n-2.0\n" "" ""
+testing "count up by zero" "seq 4 0 8 | head -n 4" "4\n4\n4\n4\n" "" ""
+testing "count nowhere by zero" "seq 4 0 4 | head -n 4" "4\n4\n4\n4\n" "" ""
+testing "count down by zero" "seq 8 0 4 | head -n 4" "" "" ""
 testing "separator -" "seq -s - 1 3" "1-2-3\n" "" ""
 testing "format string" 'seq -f %+01g -10 5 10' "-10\n-5\n+0\n+5\n+10\n" \
   "" ""
@@ -46,3 +48,18 @@ do
   testing "filter reject -f '$i'" \
 "seq -f '$i' 1 3 2>/dev/null || echo no" "no\n" "" ""
 done
+
+testing "precision inc" "seq -s, 1.0 2.00 4" "1.00,3.00\n" "" ""
+testing "precision first" "seq -s, 1.000 2.0 4" "1.000,3.000\n" "" ""
+testing "precision last" "seq -s, 1.0 2.0 4.00" "1.0,3.0\n" "" ""
+testing "precision int" "seq -s, 9007199254740991 1 9007199254740991" \
+  "9007199254740991\n" "" ""
+testing "precision e" "seq -s, 1.0e0 2" "1.0,2.0\n" "" ""
+testing "precision E" "seq -s, 1.0E0 2" "1.0,2.0\n" "" ""
+
+testing "invalid last" "seq 1 1 1f 2>/dev/null || echo y" "y\n" "" ""
+testing "invalid first" "seq 1f 1 1 2>/dev/null || echo y" "y\n" "" ""
+testing "invalid increment" "seq 1 1f 1 2>/dev/null || echo y" "y\n" "" ""
+
+# TODO: busybox fails this too, but GNU seems to not use double for large ints.
+#testing "too large for double" "seq -s, 9007199254740991 1 9007199254740992" "9007199254740992\n" "" ""
diff --git a/toys/lsb/seq.c b/toys/lsb/seq.c
index 0202c67..bcf4b74 100644
--- a/toys/lsb/seq.c
+++ b/toys/lsb/seq.c
@@ -15,11 +15,11 @@ config SEQ
 
 Count from first to last, by increment. Omitted arguments default
 to 1. Two arguments are used as first and last. Arguments can be
-negative or floating point.
+positive/negative and integer/floating point.
 
 -f	Use fmt_str as a printf-style floating point format string
 -s	Use sep_str as separator, default is a newline character
--w	Pad to equal width with leading zeroes.
+-w	Pad to equal width with leading zeroes
 */
 
 #define FOR_seq
@@ -42,51 +42,75 @@ static void insanitize(char *f)
   }
 }
 
+// Parse a numeric argument, with error-checking, and setting *prec to the
+// precision of this argument if greater than the existing value.
+static double parsef(char *what, char *s, int *prec)
+{
+  char *dp = strchr(s, '.'), *end;
+  double result;
+
+  if (dp && prec) {
+int this_prec = strlen(dp) - 1;
+char *e = strchr(dp, 'e');
+
+if (!e) e = strchr(dp, 'E');
+if (e) this_prec -= strlen(e);
+if (this_prec > *prec) *prec = this_prec;
+  }
+
+  errno = 0;
+  result = strtod(s, &end);
+  if (errno || *end) error_exit("invalid %s", what);
+  return result;
+}
+
 void seq_main(void)
 {
-  double first, increment, last, dd;
-  char *sep_str = "\n", *fmt_str = "%g";
-  int i;
+  double first = 1, increment = 1, last, dd;
+  int prec = 0, i;
 
-  // Parse command line arguments, with appropriate defaults.
-  // Note that any non-numeric arguments are treated as zero.