Merge branch 'merge_part_line'

When cleaning lines with --clean[-all], reduce allocating space
for the partial line in memory.  Previously, string concatenation
may have lead to excessive use of memory.

3 files changed, 306 insertions, 29 deletions

diff --git a/colorize.c b/colorize.c
index 498e566..06ef8fb 100644
--- a/colorize.c
+++ b/colorize.c
@@ -104,6 +104,8 @@
  && (streq (color_names[color2]->name, "none")     \
   || streq (color_names[color2]->name, "default")) \
 
+#define ALLOC_COMPLETE_PART_LINE 8
+
 #define COLOR_SEP_CHAR '/'
 
 #define DEBUG_FILE "debug.txt"
@@ -210,6 +212,10 @@ static void free_color_names (struct color_name **);
 static void process_args (unsigned int, char **, bool *, const struct color **, const char **, FILE **);
 static void process_file_arg (const char *, const char **, FILE **);
 static void read_print_stream (bool, const struct color **, const char *, FILE *);
+static void merge_print_line (bool, const struct color **, const char *, const char *, FILE *);
+static void complete_part_line (const char *, char **, FILE *);
+static bool get_next_char (char *, const char **, FILE *, bool *);
+static void save_char (char, char **, unsigned long *, size_t *);
 static void find_color_entries (struct color_name **, const struct color **);
 static void find_color_entry (const struct color_name *, unsigned int, const struct color **);
 static void print_line (bool, const struct color **, const char * const, unsigned int);
@@ -709,23 +715,10 @@ process_file_arg (const char *file_string, const char **file, FILE **stream)
     assert (*file);
 }
 
-#define MERGE_PRINT_LINE(part_line, line, flags, check_eof) do { \
-    char *current_line, *merged_line = NULL;                     \
-    if (part_line)                                               \
-      {                                                          \
-        merged_line = str_concat (part_line, line);              \
-        free_null (part_line);                                   \
-      }                                                          \
-    current_line = merged_line ? merged_line : (char *)line;     \
-    if (!check_eof || *current_line != '\0')                     \
-      print_line (bold, colors, current_line, flags);            \
-    free (merged_line);                                          \
-} while (false)
-
 static void
 read_print_stream (bool bold, const struct color **colors, const char *file, FILE *stream)
 {
-    char buf[BUF_SIZE + 1], *part_line = NULL;
+    char buf[BUF_SIZE + 1];
     unsigned int flags = 0;
 
     while (!feof (stream))
@@ -754,26 +747,151 @@ read_print_stream (bool bold, const struct color **colors, const char *file, FIL
               vfprintf_fail (formats[FMT_FILE], file, "unrecognized line ending");
             p = eol + SKIP_LINE_ENDINGS (flags);
             *eol = '\0';
-            MERGE_PRINT_LINE (part_line, line, flags, false);
+            print_line (bold, colors, line, flags);
             line = p;
           }
-        if (feof (stream)) {
-          MERGE_PRINT_LINE (part_line, line, 0, true);
-        }
-        else if (*line != '\0')
+        if (feof (stream))
           {
-            if (!clean && !clean_all) /* efficiency */
+            if (*line != '\0')
               print_line (bold, colors, line, 0);
-            else if (!part_line)
-              part_line = xstrdup (line);
+          }
+        else if (*line != '\0')
+          {
+            char *p;
+            if ((clean || clean_all) && (p = strrchr (line, '\033')))
+              merge_print_line (bold, colors, line, p, stream);
             else
-              {
-                char *merged_line = str_concat (part_line, line);
-                free (part_line);
-                part_line = merged_line;
-              }
+              print_line (bold, colors, line, 0);
+          }
+      }
+}
+
+static void
+merge_print_line (bool bold, const struct color **colors, const char *line, const char *p, FILE *stream)
+{
+    char *buf = NULL;
+    char *merged_part_line = NULL;
+    const char *part_line;
+
+    complete_part_line (p + 1, &buf, stream);
+
+    if (buf)
+      part_line = merged_part_line = str_concat (line, buf);
+    else
+      part_line = line;
+    free (buf);
+
+#ifdef TEST_MERGE_PART_LINE
+    printf ("%s", part_line);
+    free (merged_part_line);
+    exit (EXIT_SUCCESS);
+#else
+    print_line (bold, colors, part_line, 0);
+    free (merged_part_line);
+#endif
+}
+
+static void
+complete_part_line (const char *p, char **buf, FILE *stream)
+{
+    bool got_next_char = false, read_from_stream;
+    char ch;
+    unsigned long i = 0;
+    size_t size;
+
+    if (get_next_char (&ch, &p, stream, &read_from_stream))
+      {
+        if (ch == '[')
+          {
+            if (read_from_stream)
+              save_char (ch, buf, &i, &size);
+          }
+        else
+          {
+            if (read_from_stream)
+              ungetc ((int)ch, stream);
+            return; /* cancel */
+          }
+      }
+    else
+      return; /* cancel */
+
+    while (get_next_char (&ch, &p, stream, &read_from_stream))
+      {
+        if (isdigit (ch) || ch == ';')
+          {
+            if (read_from_stream)
+              save_char (ch, buf, &i, &size);
+          }
+        else /* read next character */
+          {
+            got_next_char = true;
+            break;
+          }
+      }
+
+    if (got_next_char)
+      {
+        if (ch == 'm')
+          {
+            if (read_from_stream)
+              save_char (ch, buf, &i, &size);
+          }
+        else
+          {
+            if (read_from_stream)
+              ungetc ((int)ch, stream);
+            return; /* cancel */
+          }
+      }
+    else
+      return; /* cancel */
+}
+
+static bool
+get_next_char (char *ch, const char **p, FILE *stream, bool *read_from_stream)
+{
+    if (**p == '\0')
+      {
+        int c;
+        if ((c = fgetc (stream)) != EOF)
+          {
+            *ch = (char)c;
+            *read_from_stream = true;
+            return true;
+          }
+        else
+          {
+            *read_from_stream = false;
+            return false;
           }
       }
+    else
+      {
+        *ch = **p;
+        (*p)++;
+        *read_from_stream = false;
+        return true;
+      }
+}
+
+static void
+save_char (char ch, char **buf, unsigned long *i, size_t *size)
+{
+    if (!*buf)
+      {
+        *size = ALLOC_COMPLETE_PART_LINE;
+        *buf = xmalloc (*size);
+      }
+    /* +1: effective occupied size of buffer */
+    else if ((*i + 1) == *size)
+      {
+        *size *= 2;
+        *buf = xrealloc (*buf, *size);
+      }
+    (*buf)[*i] = ch;
+    (*buf)[*i + 1] = '\0';
+    (*i)++;
 }
 
 static void
diff --git a/t/merge.t b/t/merge.t
new file mode 100755
index 0000000..b5c6dc7
--- /dev/null
+++ b/t/merge.t
@@ -0,0 +1,153 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use constant true  => 1;
+use constant false => 0;
+
+use File::Temp qw(tmpnam);
+use Test::More;
+
+# sequence, buffer sizes
+my @merge_success = (
+    [ "\e[30m",     [ 1..4 ] ],
+    [ "\e[31m",     [ 1..4 ] ],
+    [ "\e[32m",     [ 1..4 ] ],
+    [ "\e[33m",     [ 1..4 ] ],
+    [ "\e[34m",     [ 1..4 ] ],
+    [ "\e[35m",     [ 1..4 ] ],
+    [ "\e[36m",     [ 1..4 ] ],
+    [ "\e[37m",     [ 1..4 ] ],
+    [ "\e[39m",     [ 1..4 ] ],
+    [ "\e[1;30m",   [ 1..6 ] ],
+    [ "\e[1;31m",   [ 1..6 ] ],
+    [ "\e[1;32m",   [ 1..6 ] ],
+    [ "\e[1;33m",   [ 1..6 ] ],
+    [ "\e[1;34m",   [ 1..6 ] ],
+    [ "\e[1;35m",   [ 1..6 ] ],
+    [ "\e[1;36m",   [ 1..6 ] ],
+    [ "\e[1;37m",   [ 1..6 ] ],
+    [ "\e[1;39m",   [ 1..6 ] ],
+    [ "\e[40m",     [ 1..4 ] ],
+    [ "\e[41m",     [ 1..4 ] ],
+    [ "\e[42m",     [ 1..4 ] ],
+    [ "\e[43m",     [ 1..4 ] ],
+    [ "\e[44m",     [ 1..4 ] ],
+    [ "\e[45m",     [ 1..4 ] ],
+    [ "\e[46m",     [ 1..4 ] ],
+    [ "\e[47m",     [ 1..4 ] ],
+    [ "\e[49m",     [ 1..4 ] ],
+    [ "\e[0m",      [ 1..3 ] ],
+    [ "\e[m",       [ 1..2 ] ],
+    [ "\e[;;m",     [ 1..4 ] ],
+    [ "\e[123456m", [ 1    ] ], # tightly coupled to ALLOC_COMPLETE_PART_LINE
+);
+# sequence, buffer size
+my @merge_fail = (
+    [ "\e30m", 1 ], # missing bracket
+    [ "\e[am", 2 ], # not a digit nor ; nor m
+);
+# sequence
+my @buffer = (
+    "\e[30mz",
+    "\e[31mz",
+    "\e[32mz",
+    "\e[33mz",
+    "\e[34mz",
+    "\e[35mz",
+    "\e[36mz",
+    "\e[37mz",
+    "\e[39mz",
+    "\e[1;30mz",
+    "\e[1;31mz",
+    "\e[1;32mz",
+    "\e[1;33mz",
+    "\e[1;34mz",
+    "\e[1;35mz",
+    "\e[1;36mz",
+    "\e[1;37mz",
+    "\e[1;39mz",
+    "\e[40mz",
+    "\e[41mz",
+    "\e[42mz",
+    "\e[43mz",
+    "\e[44mz",
+    "\e[45mz",
+    "\e[46mz",
+    "\e[47mz",
+    "\e[49mz",
+    "\e[0mz",
+    "\e[mz",
+    "\e[;;mz",
+);
+# sequence, buffer size
+my @pushback = (
+    [ "\ezm", 1 ],
+    [ "\e[z", 2 ],
+);
+
+my $tests = 0;
+foreach (@merge_success) {
+    $tests += @{$_->[1]};
+}
+$tests += @merge_fail;
+$tests += @buffer;
+$tests += @pushback;
+
+my $source = 'colorize.c';
+my %programs;
+
+my $compile = sub
+{
+    my ($buf_size) = @_;
+    return true if exists $programs{$buf_size};
+    my $program = tmpnam();
+    return false unless system("gcc -DTEST_MERGE_PART_LINE -DBUF_SIZE=$buf_size -o $program $source") == 0;
+    $programs{$buf_size} = $program;
+    return true; # compiling succeeded
+};
+
+my $test_name = sub
+{
+    my ($sequence, $buf_size) = @_;
+    my $substr = substr($sequence, 0, $buf_size);
+    $substr   =~ s/^\e/ESC/;
+    $sequence =~ s/^\e/ESC/;
+    return "$sequence: $substr";
+};
+
+plan tests => $tests;
+
+foreach my $test (@merge_success) {
+    foreach my $buf_size (@{$test->[1]}) {
+        SKIP: {
+            skip 'compiling failed (merge part line)', 1 unless $compile->($buf_size);
+            ok(qx(echo -n "$test->[0]" | $programs{$buf_size} --clean) eq $test->[0], 'merge success: ' . $test_name->($test->[0], $buf_size));
+        }
+    }
+}
+foreach my $test (@merge_fail) {
+    my $buf_size = $test->[1];
+    SKIP: {
+        skip 'compiling failed (merge part line)', 1 unless $compile->($buf_size);
+        ok(qx(echo -n "$test->[0]" | $programs{$buf_size} --clean) eq substr($test->[0], 0, $buf_size), 'merge fail: ' . $test_name->($test->[0], $buf_size));
+    }
+}
+foreach my $test (@buffer) {
+    my $buf_size = length($test) - 1;
+    SKIP: {
+        skip 'compiling failed (merge part line)', 1 unless $compile->($buf_size);
+        ok(qx(echo -n "$test" | $programs{$buf_size} --clean) eq substr($test, 0, $buf_size), 'buffer: ' . $test_name->($test, $buf_size));
+    }
+}
+foreach my $test (@pushback) {
+    my $buf_size = $test->[1];
+    SKIP: {
+        my $program = tmpnam();
+        skip 'compiling failed (merge part line)', 1 unless system("gcc -DBUF_SIZE=$buf_size -o $program $source") == 0;
+        ok(qx(echo -n "$test->[0]" | $program --clean) eq $test->[0], 'pushback: ' . $test_name->($test->[0], $buf_size));
+        unlink $program;
+    }
+}
+
+unlink $programs{$_} foreach keys %programs;
diff --git a/test.pl b/test.pl
index 959d83f..30c1fd1 100755
--- a/test.pl
+++ b/test.pl
@@ -8,6 +8,7 @@ use constant false => 0;
 use File::Temp qw(tempfile tempdir tmpnam);
 use IPC::Open3 qw(open3);
 use Symbol qw(gensym);
+use Test::Harness qw(runtests);
 use Test::More;
 
 my $tests = 25;
@@ -30,6 +31,11 @@ my $write_to_tmpfile = sub
     return $tmpfile;
 };
 
+{
+    my @test_files = glob('t/*.t');
+    runtests(@test_files);
+}
+
 plan tests => $tests;
 
 SKIP: {
@@ -139,9 +145,9 @@ SKIP: {
 
         my $switch = "--$type";
 
-        # Check that line chunks are merged when cleaning text
+        # Check that line chunks are printed when cleaning text without sequences
         my $short_text = 'Linux dev 2.6.32-5-openvz-686 #1 SMP Sun Sep 23 11:40:07 UTC 2012 i686 GNU/Linux';
-        is(qx(echo -n "$short_text" | $program_buf $switch), $short_text, "merge ${\length $short_text} bytes (BUF_SIZE=$BUF_SIZE{short}, $type)");
+        is(qx(echo -n "$short_text" | $program_buf $switch), $short_text, "print ${\length $short_text} bytes (BUF_SIZE=$BUF_SIZE{short}, $type)");
     };
 
     SKIP: {