Subject: Re: bug#13525: 8.20 - [seq 1 3 1] treated as [seq 1 3]

From: Pádraig Brady <[email-masked]>
Date: Tue, 22 Jan 2013 12:59:19 +0000

On 01/22/2013 12:17 PM, Pádraig Brady wrote:
> On 01/22/2013 11:23 AM, Pádraig Brady wrote:
>> On 01/22/2013 10:43 AM, Marcel Böhme wrote:
>>>
>>> Dear all,
>>>
>>> There is another bug that sneaked into the speed patch of seq on 13.09.12:
>>>
>>> 560   if (all_digits_p (argv[optind])
>>> 561       && (n_args == 1 || all_digits_p (argv[optind + 1]))
>>> 562       && (n_args < 3 || STREQ ("1", argv[optind + 2]))
>>> 563       && !equal_width && !format_str && strlen (separator) == 1)
>>>
>>> That should probably be:
>>> 560   if (all_digits_p (argv[optind])
>>> 561       && (n_args == 1 || all_digits_p (argv[optind + 1]))
>>> 562       && (n_args < 3 || STREQ ("1", argv[optind + 1]))
>>> 563       && !equal_width && !format_str && strlen (separator) == 1)
>>
>> Sigh we really messed up seq in that release :(
>> The attached should fix it:

The seq 0 1 0 "regression" was only because I didn't
adjust for the parameters passed into seq_fast.

So hopefully this update caters for all cases.

thanks,
Pádraig.
>From 98a1ebe873e318dcc94ebb984ef496329d349f0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Tue, 22 Jan 2013 11:13:16 +0000
Subject: [PATCH] seq: fix to always honor the step value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/seq.c (main): With 3 positive integer args we were
checking the end value was == "1", rather than the step value.
* tests/misc/seq.pl: Add tests for this case.
Reported by Marcel Böhme in http://bugs.gnu.org/13525
---
 NEWS              |    3 ++-
 src/seq.c         |    5 +++--
 tests/misc/seq.pl |   11 +++++++++++
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 46d1aba..73b49c3 100644
--- a/NEWS
+++ b/NEWS
@@ -49,7 +49,8 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   seq -s no longer prints an erroneous newline after the first number, and
   outputs a newline after the last number rather than a trailing separator.
-  [bug introduced in coreutils-8.20]
+  Also seq no longer ignores a specified step value when the end value is 1.
+  [bugs introduced in coreutils-8.20]
 
 ** Changes in behavior
 
diff --git a/src/seq.c b/src/seq.c
index e1b467c..22126a2 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -559,11 +559,12 @@ main (int argc, char **argv)
      then use the much more efficient integer-only code.  */
   if (all_digits_p (argv[optind])
       && (n_args == 1 || all_digits_p (argv[optind + 1]))
-      && (n_args < 3 || STREQ ("1", argv[optind + 2]))
+      && (n_args < 3 || (STREQ ("1", argv[optind + 1])
+                         && all_digits_p (argv[optind + 2])))
       && !equal_width && !format_str && strlen (separator) == 1)
     {
       char const *s1 = n_args == 1 ? "1" : argv[optind];
-      char const *s2 = n_args == 1 ? argv[optind] : argv[optind + 1];
+      char const *s2 = argv[optind + (n_args - 1)];
       if (seq_fast (s1, s2))
         exit (EXIT_SUCCESS);
 
diff --git a/tests/misc/seq.pl b/tests/misc/seq.pl
index 40a7571..205c4d2 100755
--- a/tests/misc/seq.pl
+++ b/tests/misc/seq.pl
@@ -133,6 +133,17 @@ my @Tests =
    ['sep-1', qw(-s, 1 3), {OUT => [qw(1,2,3)]}],
    ['sep-2', qw(-s, 1 1), {OUT => [qw(1)]}],
    ['sep-3', qw(-s,, 1 3), {OUT => [qw(1,,2,,3)]}],
+
+   # Exercise fast path avoidance logic.
+   # In 8.20 a step value != 1, with positive integer start and end was broken
+   ['not-fast-1', qw(1 3 1), {OUT => [qw(1)]}],
+   ['not-fast-2', qw(1 1 4.2), {OUT => [qw(1 2 3 4)]}],
+   ['not-fast-3', qw(1 1 0)],
+
+   # Ensure the correct parameters are passed to the fast path
+   ['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)]}],
   );
 
 # Append a newline to each entry in the OUT array.
-- 
1.7.6.4