[Toybox] [PATCH] Fix various seq bugs.

[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(-)

[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<bits<64 integers before
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 1000000 because of %g.
increasing that to 53 bits should be enough for anyone... :-)

Post by 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(-)

[enh]

ping?

Post by 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<bits<64 integers before
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 1000000 because of %g.
increasing that to 53 bits should be enough for anyone... :-)

Post by 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(-)

--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.

[Rob Landley]

Post by enh
ping?

Sigh. I had a message half-composed when my netbook crashed last week,

Post by enh
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 9999 | tail -n 3
9997.000
9998.000
9999.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

[Josh Gao]

Post by Rob Landley
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 1000000 1000001
1e+06
1e+06

[Rob Landley]

Post by Rob Landley
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?
$ seq 1000000 1000001
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...

[Rob Landley]

Post by Rob Landley

Post by Rob Landley
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?
$ seq 1000000 1000001
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...)
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
+++ actual 2017-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

[enh]

Post by Rob Landley

Post by Rob Landley
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?
$ seq 1000000 1000001
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.

Post by Rob Landley
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
http://lists.landley.net/listinfo.cgi/toybox-landley.net

[Rob Landley]

Post by enh

Post by Rob Landley

Post by Rob Landley
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?
$ seq 1000000 1000001
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.

Post by enh
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