Subject: Re: bug#23110: seq apparent bug

From: Bernhard Voelker <[email-masked]>
Date: Thu, 14 Apr 2016 18:37:51 +0200

On 04/07/2016 08:18 PM, Pádraig Brady wrote:
> I agree that we should avoid repeating output with "0" STEP.

Thanks.  I improved the error diagnostic by outputting the original
value from the user:

[PATCH 1/2] seq: do not allow 0 as increment value

> Do we want to deal with these cases spinning the cpu?
> 
>    seq 1 nan 1

This is addressed with the second patch:

[PATCH 2/2] seq: do not allow NaN arguments

>    seq 1 .0000000000000000000000000000001 1

I consider this a bug in seq: from mathematical point of view, the above
should just output "1" and then exit, because after adding that tiny number
the next number would be greater than LAST.

IMO we should enhance seq_fast() to do all the Math when no special
output format is given.
WDYT?

> As an aside, I see FreeBSD requires the STEP to be in the right direction
> when FIRST != LAST, or it will also exit with error.
> GNU will just output nothing in that case.

I think GNU seq's behavior is okay, and therefore I agree with Paul in
http://lists.gnu.org/archive/html/bug-coreutils/2016-04/msg00032.html

Have a nice day,
Berny
>From 81e589021d9c47e4fbc4284e82881a9703246476 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email-masked]>
Date: Thu, 14 Apr 2016 12:38:09 +0200
Subject: [PATCH 1/2] seq: do not allow 0 as increment value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/seq.c (main): Exit with an error diagnostic when the given
step value is Zero.
(usage): Document it.
* doc/coreutils.texi (seq invocation): Likewise.
* tests/misc/seq.pl: Add tests.
* NEWS (Changes in behavior): Mention the change.
Reported by Маренков Евгений in:
http://bugs.gnu.org/23110
---
 NEWS               |  2 ++
 doc/coreutils.texi |  2 ++
 src/seq.c          |  8 ++++++++
 tests/misc/seq.pl  | 10 ++++++++++
 4 files changed, 22 insertions(+)

diff --git a/NEWS b/NEWS
index 9445977..13af702 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,8 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Changes in behavior
 
+   seq no longer accepts 0 value as increment argument.
+
    stat now outputs nanosecond information for time stamps even if
    they are out of localtime range.
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 45706bd..6b70635 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17422,6 +17422,8 @@ even when @var{first} is larger than @var{last}.
 The sequence of numbers ends when the sum of the current number and
 @var{increment} would become greater than @var{last},
 so @code{seq 1 10 10} only produces @samp{1}.
+@var{increment} must not be @samp{0}; use @command{yes} to get
+repeated output of a constant number.
 Floating-point numbers may be specified.  @xref{Floating point}.
 
 The program accepts the following options.  Also see @ref{Common options}.
diff --git a/src/seq.c b/src/seq.c
index fbb94a0..91cf625 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -92,6 +92,7 @@ INCREMENT would become greater than LAST.\n\
 FIRST, INCREMENT, and LAST are interpreted as floating point values.\n\
 INCREMENT is usually positive if FIRST is smaller than LAST, and\n\
 INCREMENT is usually negative if FIRST is greater than LAST.\n\
+INCREMENT must not be 0.\n\
 "), stdout);
       fputs (_("\
 FORMAT must be suitable for printing one argument of type 'double';\n\
@@ -635,6 +636,13 @@ main (int argc, char **argv)
       if (optind < argc)
         {
           step = last;
+          if (step.value == 0)
+            {
+              error (0, 0, _("invalid Zero increment value: %s"),
+                     quote (argv[optind-1]));
+              usage (EXIT_FAILURE);
+            }
+
           last = scan_arg (argv[optind++]);
         }
     }
diff --git a/tests/misc/seq.pl b/tests/misc/seq.pl
index 6564415..ac0664f 100755
--- a/tests/misc/seq.pl
+++ b/tests/misc/seq.pl
@@ -25,6 +25,7 @@ use strict;
 
 my $prog = 'seq';
 my $try_help = "Try '$prog --help' for more information.\n";
+my $err_inc_zero = "seq: invalid Zero increment value: '0'\n".$try_help;
 
 my $locale = $ENV{LOCALE_FR_UTF8};
 ! defined $locale || $locale eq 'none'
@@ -151,6 +152,15 @@ my @Tests =
    ['fast-1', qw(4), {OUT => [qw(1 2 3 4)]}],
    ['fast-2', qw(1 4), {OUT => [qw(1 2 3 4)]}],
    ['fast-3', qw(1 1 4), {OUT => [qw(1 2 3 4)]}],
+
+   # Ensure an INCREMENT of Zero is rejected.
+   ['inc-zero-1',	qw(1 0 10), {EXIT => 1}, {ERR => $err_inc_zero}],
+   ['inc-zero-2',	qw(0 -0 0), {EXIT => 1}, {ERR => $err_inc_zero},
+    {ERR_SUBST => 's/-0/0/'}],
+   ['inc-zero-3',	qw(1 0.0 10), {EXIT => 1},{ERR => $err_inc_zero},
+    {ERR_SUBST => 's/0.0/0/'}],
+   ['inc-zero-4',	qw(1 -0.0e-10 10), {EXIT => 1},{ERR => $err_inc_zero},
+    {ERR_SUBST => 's/-0\.0e-10/0/'}],
   );
 
 # Append a newline to each entry in the OUT array.
-- 
2.1.4

>From 9a2e8ac489d7f7b2af4e1468f6174db9106eb9e4 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email-masked]>
Date: Thu, 14 Apr 2016 12:39:28 +0200
Subject: [PATCH 2/2] seq: do not allow NaN arguments

* src/seq.c (isnan): Define macro.
(scan_arg): Add check if the given argument is NaN, and exit with
a proper error diagnostic in such a case.
(usage): Document it.
* tests/misc/seq.pl: Add tests.
* doc/coreutils.texi (seq invocation): Document the change.
* NEWS (Changes in behavior): Mention the change.
---
 NEWS               |  3 ++-
 doc/coreutils.texi |  1 +
 src/seq.c          | 14 ++++++++++++--
 tests/misc/seq.pl  | 17 +++++++++++++++++
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 13af702..4cb5caf 100644
--- a/NEWS
+++ b/NEWS
@@ -17,7 +17,8 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Changes in behavior
 
-   seq no longer accepts 0 value as increment argument.
+   seq no longer accepts 0 value as increment, and now also rejects NaN
+   values for any argument.
 
    stat now outputs nanosecond information for time stamps even if
    they are out of localtime range.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 6b70635..5630201 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17424,6 +17424,7 @@ The sequence of numbers ends when the sum of the current number and
 so @code{seq 1 10 10} only produces @samp{1}.
 @var{increment} must not be @samp{0}; use @command{yes} to get
 repeated output of a constant number.
+@var{first}, @var{increment} and @var{last} must not be @code{NaN}.
 Floating-point numbers may be specified.  @xref{Floating point}.
 
 The program accepts the following options.  Also see @ref{Common options}.
diff --git a/src/seq.c b/src/seq.c
index 91cf625..de92bc2 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -27,11 +27,14 @@
 #include "quote.h"
 #include "xstrtod.h"
 
-/* Roll our own isfinite rather than using <math.h>, so that we don't
+/* Roll our own isfinite/isnan rather than using <math.h>, so that we don't
    have to worry about linking -lm just for isfinite.  */
 #ifndef isfinite
 # define isfinite(x) ((x) * 0 == 0)
 #endif
+#ifndef isnan
+# define isnan(x) ((x) != (x))
+#endif
 
 /* The official name of this program (e.g., no 'g' prefix).  */
 #define PROGRAM_NAME "seq"
@@ -92,7 +95,7 @@ INCREMENT would become greater than LAST.\n\
 FIRST, INCREMENT, and LAST are interpreted as floating point values.\n\
 INCREMENT is usually positive if FIRST is smaller than LAST, and\n\
 INCREMENT is usually negative if FIRST is greater than LAST.\n\
-INCREMENT must not be 0.\n\
+INCREMENT must not be 0; none of FIRST, INCREMENT and LAST may be NaN.\n\
 "), stdout);
       fputs (_("\
 FORMAT must be suitable for printing one argument of type 'double';\n\
@@ -144,6 +147,13 @@ scan_arg (const char *arg)
       usage (EXIT_FAILURE);
     }
 
+  if (isnan (ret.value))
+    {
+      error (0, 0, _("invalid %s argument: %s"), quote_n (0, "not-a-number"),
+             quote_n (1, arg));
+      usage (EXIT_FAILURE);
+    }
+
   /* We don't output spaces or '+' so don't include in width */
   while (isspace (to_uchar (*arg)) || *arg == '+')
     arg++;
diff --git a/tests/misc/seq.pl b/tests/misc/seq.pl
index ac0664f..130bbf3 100755
--- a/tests/misc/seq.pl
+++ b/tests/misc/seq.pl
@@ -26,6 +26,7 @@ use strict;
 my $prog = 'seq';
 my $try_help = "Try '$prog --help' for more information.\n";
 my $err_inc_zero = "seq: invalid Zero increment value: '0'\n".$try_help;
+my $err_nan_arg = "seq: invalid 'not-a-number' argument: 'nan'\n".$try_help;
 
 my $locale = $ENV{LOCALE_FR_UTF8};
 ! defined $locale || $locale eq 'none'
@@ -161,6 +162,22 @@ my @Tests =
     {ERR_SUBST => 's/0.0/0/'}],
    ['inc-zero-4',	qw(1 -0.0e-10 10), {EXIT => 1},{ERR => $err_inc_zero},
     {ERR_SUBST => 's/-0\.0e-10/0/'}],
+
+   # Ensure NaN arguments rejected.
+   ['nan-first-1', qw(nan),       {EXIT => 1}, {ERR => $err_nan_arg}],
+   ['nan-first-2', qw(NaN 2),     {EXIT => 1}, {ERR => $err_nan_arg},
+    {ERR_SUBST => 's/NaN/nan/'}],
+   ['nan-first-3', qw(nan 1 2),   {EXIT => 1}, {ERR => $err_nan_arg}],
+   ['nan-first-4', qw(-- -nan),   {EXIT => 1}, {ERR => $err_nan_arg},
+    {ERR_SUBST => 's/-nan/nan/'}],
+   ['nan-inc-1',   qw(1 nan 2),   {EXIT => 1}, {ERR => $err_nan_arg}],
+   ['nan-inc-2',   qw(1 -NaN 2),  {EXIT => 1}, {ERR => $err_nan_arg},
+    {ERR_SUBST => 's/-NaN/nan/'}],
+   ['nan-last-1',  qw(1 1 nan),   {EXIT => 1}, {ERR => $err_nan_arg}],
+   ['nan-last-2',  qw(1 NaN),     {EXIT => 1}, {ERR => $err_nan_arg},
+    {ERR_SUBST => 's/NaN/nan/'}],
+   ['nan-last-3',  qw(0 -1 -NaN), {EXIT => 1}, {ERR => $err_nan_arg},
+    {ERR_SUBST => 's/-NaN/nan/'}],
   );
 
 # Append a newline to each entry in the OUT array.
-- 
2.1.4