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