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 <e...@google.com>
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.
-  first = increment = 1;
   switch (toys.optc) {
-    case 3: increment = atof(toys.optargs[1]);
-    case 2: first = atof(*toys.optargs);
-    default: last = atof(toys.optargs[toys.optc-1]);
+    case 3: increment = parsef("increment", toys.optargs[1], &prec);
+    case 2: first = parsef("first", *toys.optargs, &prec);
+    default: last = parsef("last", toys.optargs[toys.optc-1], NULL);
   }
 
+  // Were we given a format string, or do we have to create one based
+  // on our inputs? For some reason GNU and busybox ignore the precision
+  // of `last`, and only use `first` and `increment`.
+  if (toys.optflags & FLAG_f) insanitize(TT.fmt);
+  else sprintf(TT.fmt = toybuf, "%%.%df", prec);
+
   // Pad to largest width
   if (toys.optflags & FLAG_w) {
     char *s;
-    int len, dot, left = 0, right = 0;
+    int len, dot, left = 0;
 
     for (i=0; i<3; i++) {
       dd = (double []){first, increment, last}[i];
 
-      len = sprintf(toybuf, "%g", dd);
-      if ((s = strchr(toybuf, '.'))) {
-        dot = s-toybuf;
+      len = sprintf(toybuf+128, TT.fmt, dd);
+      if ((s = strchr(toybuf+128, '.'))) {
+        dot = s-(toybuf+128);
         if (left<dot) left = dot;
-        dot = len-dot-1;
-        if (right<dot) right = dot;
       } else if (len>left) left = len;
     }
 
-    sprintf(fmt_str = toybuf, "%%0%d.%df", left+right+!!right, right);
+    sprintf(TT.fmt = toybuf, "%%0%d.%df", left+prec+!!prec, prec);
   }
-  if (toys.optflags & FLAG_f) insanitize(fmt_str = TT.fmt);
-  if (toys.optflags & FLAG_s) sep_str = TT.sep;
+  if (!(toys.optflags & FLAG_s)) TT.sep = "\n";
+
+  // If increment is 0, GNU and busybox loop forever if first<=last, but
+  // output nothing if first>last.
+  if (increment == 0.0 && first>last) return;
 
   i = 0;
-  dd = first;
-  if (increment) for (;;) {
-    // avoid accumulating rounding errors from increment
+  for (;;) {
+    // Multiply to avoid accumulating rounding errors from increment.
     dd = first+i*increment;
     if ((increment<0 && dd<last) || (increment>0 && dd>last)) break;
-    if (i++) printf("%s", sep_str);
-    printf(fmt_str, dd);
+    if (i++) printf("%s", TT.sep);
+    printf(TT.fmt, dd);
   }
 
   if (i) printf("\n");
-- 
2.13.0.rc2.291.g57267f2277-goog

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

Reply via email to