Subject: bug#42764: closed (Re: bug#42764: csplit does not suppress the last match when not using {*})

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 08 Aug 2020 20:58:02 +0000

Your bug report

#42764: csplit does not suppress the last match when not using {*}

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
42764: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=42764
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#42764: csplit does not suppress the last match when not using {*}
From: Pádraig Brady <[email-masked]>
To: Emanuele Giacomelli <[email-masked]>, [email-masked]
Date: Sat, 8 Aug 2020 21:56:48 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

On 08/08/2020 10:12, Emanuele Giacomelli via GNU coreutils Bug Reports wrote:
> Good day,
> 
> I am experiencing an odd behaviour in csplit which may actually be a
> bug.
> 
> I am testing this against the code cloned from
> https://github.com/coreutils/coreutils.git, on the commit described by
> git as v8.32-52-gc0e5f8c59.
> 
> Suppose I have the following YAML file:
> 
> ==> test.yaml <==
> value1: 123
> ---
> value2: 456
> ---
> value3: 789
> 
> and I want to split it at '---' lines. First I would try the following:
> 
>      csplit -z --suppress-matched test.yaml '/^---$/' '{1}'
> 
> which outputs:
> 
>      12
>      12
>      16
> 
> and creates the following files:
> 
>      ==> xx00 <==
>      value1: 123
> 
>      ==> xx01 <==
>      value2: 456
> 
>      ==> xx02 <==
>      ---
>      value3: 789
> 
> The last portion still contains the '---', despite it being suppressed
> from the second part.
> 
> Now, if I try again with:
> 
>      csplit -z --suppress-matched test.yaml '/^---$/' '{*}'
> 
> I get:
> 
>      12
>      12
>      12
> 
> and:
> 
>      ==> xx00 <==
>      value1: 123
> 
>      ==> xx01 <==
>      value2: 456
> 
>      ==> xx02 <==
>      value3: 789
> 
> where the last part does not contain the matched line, as expected.
> 
> While trying to figure out the problem, I noticed that match suppression
> is done at the beginning of process_regexp. For a match-twice scenario
> like the first one, the function is called twice, then the rest of the
> file is simply dumped by split_file.
> 
> This means that the two calls to process_regexp will:
> 
> * suppress nothing for call #1 because nothing has been matched yet;
> * suppress the first match in call #2.
> 
> Then, the rest of the file is dumped but no one actually suppressed the
> second match, which appears in the last segment. When using asterisk
> repetition, the file is instead dumped by process_regexp, which gets its
> chance to suppress the matched line.
> 
> I came up with the attached patch, which simply moves match suppression
> at the end of process_regexp. With this modification, the invocation:
> 
>      csplit -z --suppress-matched test.yaml '/^---$/' '{1}'
> 
> now produces:
> 
>      12
>      12
>      12
> 
> and:
> 
> ==> xx00 <==
> value1: 123
> 
> ==> xx01 <==
> value2: 456
> 
> ==> xx02 <==
> value3: 789
> 
> which is what I would expect.
> 

I agree with this analysis.
The usual manifestation would probably be
when there was only a single match.
I.E. when not specifying a repetition count,
we were not suppressing the single match.

I'll apply the attached in your name later today
(which also adds a test).

Marking this as done.

thanks!
Pádraig
>From 7cf45f4f6a093a927d3139c87f52999dd2c750ec Mon Sep 17 00:00:00 2001
From: Emanuele Giacomelli <[email-masked]>
Date: Sat, 8 Aug 2020 21:29:13 +0100
Subject: [PATCH] csplit: fix regex suppression with specific match count

* src/csplit.c (process_regexp): Process the line suppression
in all invocations so that the last match is suppressed.
Previously with a non infinite match count,
the last regex pattern was not suppressed.
* NEWS: Mention the bug fix.
* tests/misc/csplit-suppress-matched.pl: Add a test case.
Fixes https://bugs.gnu.org/42764
---
 NEWS                                  |  4 ++++
 src/csplit.c                          |  6 +++---
 tests/misc/csplit-suppress-matched.pl | 12 +++++++++---
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 1881de115..61b711611 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   is a non regular file.
   [bug introduced in coreutils-8.6]
 
+  csplit --suppress-matched now elides the last matched line
+  when a specific number of pattern matches are performed.
+  [bug introduced with the --suppress-matched feature in coreutils-8.22]
+
   du no longer crashes on XFS file systems when the directory hierarchy is
   heavily changed during the run.
   [bug introduced in coreutils-8.25]
diff --git a/src/csplit.c b/src/csplit.c
index 9bd9c43b5..93ff60dc6 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -803,9 +803,6 @@ process_regexp (struct control *p, uintmax_t repetition)
   if (!ignore)
     create_output_file ();
 
-  if (suppress_matched && current_line > 0)
-    remove_line ();
-
   /* If there is no offset for the regular expression, or
      it is positive, then it is not necessary to buffer the lines. */
 
@@ -893,6 +890,9 @@ process_regexp (struct control *p, uintmax_t repetition)
 
   if (p->offset > 0)
     current_line = break_line;
+
+  if (suppress_matched)
+    remove_line ();
 }
 
 /* Split the input file according to the control records we have built. */
diff --git a/tests/misc/csplit-suppress-matched.pl b/tests/misc/csplit-suppress-matched.pl
index 80f5299d0..e15ebb0f2 100755
--- a/tests/misc/csplit-suppress-matched.pl
+++ b/tests/misc/csplit-suppress-matched.pl
@@ -67,21 +67,27 @@ my @csplit_tests =
     {OUTPUTS => [ "a\na\nYY\n", "\nXX\nb\nb\nYY\n","\nXX\nc\nYY\n",
                   "\nXX\nd\nd\nd\n" ] }],
 
-  # the newline (matched line) does not appears in the output files
+  # the newline (matched line) does not appear in the output files
   ["re-1", " --suppress-matched -q - '/^\$/' '{*}'", {IN_PIPE => $IN_UNIQ},
     {OUTPUTS => ["a\na\nYY\n", "XX\nb\nb\nYY\n", "XX\nc\nYY\n",
                  "XX\nd\nd\nd\n"]}],
 
-  # the 'XX' (matched line + offset 1) does not appears in the output files.
+  # the 'XX' (matched line + offset 1) does not appear in the output files.
   # the newline appears in the files (before each split, at the end of the file)
   ["re-2", "--suppress-matched -q - '/^\$/1' '{*}'", {IN_PIPE => $IN_UNIQ},
     {OUTPUTS => ["a\na\nYY\n\n","b\nb\nYY\n\n","c\nYY\n\n","d\nd\nd\n"]}],
 
-  # the 'YY' (matched line + offset of -1) does not appears in the output files
+  # the 'YY' (matched line + offset of -1) does not appear in the output files
   # the newline appears in the files (as the first line of the new split)
   ["re-3", " --suppress-matched -q - '/^\$/-1' '{*}'", {IN_PIPE => $IN_UNIQ},
     {OUTPUTS => ["a\na\n", "\nXX\nb\nb\n", "\nXX\nc\n", "\nXX\nd\nd\nd\n"]}],
 
+  # the last matched line for a non infinite match repetition is suppressed.
+  # Up to and including coreutils 8.32, the last match was output.
+  ["re-4", " --suppress-matched -q - '/^\$/' '{2}'", {IN_PIPE => $IN_UNIQ},
+    {OUTPUTS => ["a\na\nYY\n", "XX\nb\nb\nYY\n", "XX\nc\nYY\n",
+                 "XX\nd\nd\nd\n"]}],
+
   # Test two consecutive matched lines
   # without suppress-matched, the second file should contain a single newline.
   ["re-4.1", "-q - '/^\$/' '{*}'", {IN_PIPE => "a\n\n\nb\n"},
-- 
2.26.2

Subject: csplit does not suppress the last match when not using {*}
From: Emanuele Giacomelli <[email-masked]>
To: "[email-masked]" <[email-masked]>
Date: Sat, 8 Aug 2020 09:12:51 +0000 (UTC)
List-Id: <debbugs-submit.debbugs.gnu.org>

Good day,

I am experiencing an odd behaviour in csplit which may actually be a
bug.

I am testing this against the code cloned from
https://github.com/coreutils/coreutils.git, on the commit described by
git as v8.32-52-gc0e5f8c59.

Suppose I have the following YAML file:

==> test.yaml <==
value1: 123
---
value2: 456
---
value3: 789

and I want to split it at '---' lines. First I would try the following:

    csplit -z --suppress-matched test.yaml '/^---$/' '{1}'

which outputs:

    12
    12
    16

and creates the following files:

    ==> xx00 <==
    value1: 123

    ==> xx01 <==
    value2: 456

    ==> xx02 <==
    ---
    value3: 789

The last portion still contains the '---', despite it being suppressed
from the second part.

Now, if I try again with:

    csplit -z --suppress-matched test.yaml '/^---$/' '{*}'

I get:

    12
    12
    12

and:

    ==> xx00 <==
    value1: 123

    ==> xx01 <==
    value2: 456

    ==> xx02 <==
    value3: 789

where the last part does not contain the matched line, as expected.

While trying to figure out the problem, I noticed that match suppression
is done at the beginning of process_regexp. For a match-twice scenario
like the first one, the function is called twice, then the rest of the
file is simply dumped by split_file.

This means that the two calls to process_regexp will:

* suppress nothing for call #1 because nothing has been matched yet;
* suppress the first match in call #2.

Then, the rest of the file is dumped but no one actually suppressed the
second match, which appears in the last segment. When using asterisk
repetition, the file is instead dumped by process_regexp, which gets its
chance to suppress the matched line.

I came up with the attached patch, which simply moves match suppression
at the end of process_regexp. With this modification, the invocation:

    csplit -z --suppress-matched test.yaml '/^---$/' '{1}'

now produces:

    12
    12
    12

and:

==> xx00 <==
value1: 123

==> xx01 <==
value2: 456

==> xx02 <==
value3: 789

which is what I would expect.

diff --git a/src/csplit.c b/src/csplit.c
index 9bd9c43b5..93ff60dc6 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -803,9 +803,6 @@ process_regexp (struct control *p, uintmax_t repetition)
   if (!ignore)
     create_output_file ();
 
-  if (suppress_matched && current_line > 0)
-    remove_line ();
-
   /* If there is no offset for the regular expression, or
      it is positive, then it is not necessary to buffer the lines. */
 
@@ -893,6 +890,9 @@ process_regexp (struct control *p, uintmax_t repetition)
 
   if (p->offset > 0)
     current_line = break_line;
+
+  if (suppress_matched)
+    remove_line ();
 }
 
 /* Split the input file according to the control records we have built. */