bug#10953: Potential logical bug in readtokens.c

[Jim Meyering]

Paul Eggert wrote:
> Thanks, I agree that code is potentially buggy.  I don't see
> any way to trigger the bug in coreutils, but it's just asking
> for trouble.  Here's a proposed patch.

Hi Paul,
Thanks for working on that.
I had applied, tested and reviewed it earlier today,
as I was writing the changes below.

I've just noticed that I pushed it along with the quotearg/quote.h fix
that I pushed earlier today.  Must remember always to put under-review
change-sets on separate topic branch, not on master.  Sorry for the lapse.

>>From 4954a3517397dadd217d6244e961dd855fbadbef Mon Sep 17 00:00:00 2001
> From: Paul Eggert <address@hidden>
> Date: Tue, 6 Mar 2012 15:19:24 -0800
> Subject: [PATCH] readtokens: avoid core dumps with unusual calling patterns
>
> Reported by Xu Zhongxing in <http://debbugs.gnu.org/10953>.
> * lib/readtokens.c: Include limits.h.
> (word, bits_per_word, get_nth_bit, set_nth_bit): New.
> (readtoken): Don't cache the delimiters; the cache code was buggy
> if !delim && saved_delim, or if the new n_delim differs from the old.
> Also, it wasn't thread-safe.

Here's a proposed readtokens test module:

>From 52605d539a37d4d303e7863d946e85bef21bbcc3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 7 Mar 2012 12:16:30 +0100
Subject: [PATCH] readtokens: add tests

* modules/readtokens-tests: New file.
* tests/test-readtokens.c: New file.
* tests/test-readtokens.sh: New file.
---
 ChangeLog                |    6 +++
 modules/readtokens-tests |   13 ++++++
 tests/test-readtokens.c  |   98 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/test-readtokens.sh |   23 +++++++++++
 4 files changed, 140 insertions(+)
 create mode 100644 modules/readtokens-tests
 create mode 100644 tests/test-readtokens.c
 create mode 100755 tests/test-readtokens.sh

diff --git a/ChangeLog b/ChangeLog
index 8ac1ba4..83a09a4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2012-03-07  Jim Meyering  <address@hidden>

+       readtokens: add tests
+       * modules/readtokens-tests: New file.
+       * tests/test-readtokens.c: New file.
+
+2012-03-07  Jim Meyering  <address@hidden>
+
        quotearg: the module must now include quote.h
        With commit v0.0-7133-g6417476, quotearg.c includes "quote.h".
        So must the module.
diff --git a/modules/readtokens-tests b/modules/readtokens-tests
new file mode 100644
index 0000000..a1982dd
--- /dev/null
+++ b/modules/readtokens-tests
@@ -0,0 +1,13 @@
+Files:
+tests/macros.h
+tests/test-readtokens.c
+tests/test-readtokens.sh
+
+Depends-on:
+closeout
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-readtokens.sh
+check_PROGRAMS += test-readtokens
diff --git a/tests/test-readtokens.c b/tests/test-readtokens.c
new file mode 100644
index 0000000..b0af5c8
--- /dev/null
+++ b/tests/test-readtokens.c
@@ -0,0 +1,98 @@
+/* Test the readtokens module.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "readtokens.h"
+#include "closeout.h"
+#include "macros.h"
+
+static void
+basic (void)
+{
+  char *filename = "in.827";
+  int fd = open (filename, O_CREAT | O_WRONLY, 0600);
+  ASSERT (fd >= 0);
+  ASSERT (write (fd, "a|b;c+d", 7) == 7);
+  ASSERT (close (fd) == 0);
+
+  {
+    token_buffer tb;
+    FILE *fp = fopen (filename, "r");
+    ASSERT (fp);
+
+    init_tokenbuffer (&tb);
+    ASSERT (readtoken (fp, "|;", 2, &tb)  == 1 && tb.buffer[0] == 'a');
+    ASSERT (readtoken (fp, "|;", 2, &tb) == 1 && tb.buffer[0] == 'b');
+    ASSERT (readtoken (fp, "+", 1, &tb)  == 1 && tb.buffer[0] == 'c');
+    ASSERT (readtoken (fp, "-", 1, &tb) == 1 && tb.buffer[0] == 'd');
+    ASSERT (readtoken (fp, "%", 0, &tb) == (size_t) -1);
+    ASSERT ( ! ferror (fp));
+    ASSERT (fclose (fp) == 0);
+  }
+}
+
+int
+main (int argc, char **argv)
+{
+  token_buffer tb;
+  char *delim;
+  size_t delim_len;
+  bool ok = true;
+
+  atexit (close_stdout);
+
+  if (argc == 1)
+    {
+      basic ();
+      return 0;
+    }
+
+  init_tokenbuffer (&tb);
+
+  if (argc != 2)
+    return 99;
+
+  delim = argv[1];
+  delim_len = strlen (delim);
+
+  if (STREQ (delim, "\\0"))
+    {
+      delim = "";
+      delim_len = 1;
+    }
+
+  while (1)
+    {
+      size_t token_length = readtoken (stdin, delim, delim_len, &tb);
+      if (token_length == (size_t) -1)
+        break;
+      fwrite (tb.buffer, 1, token_length, stdout);
+      putchar (':');
+    }
+  putchar ('\n');
+  free (tb.buffer);
+
+  ASSERT ( ! ferror (stdin));
+
+  return 0;
+}
diff --git a/tests/test-readtokens.sh b/tests/test-readtokens.sh
new file mode 100755
index 0000000..51fd41e
--- /dev/null
+++ b/tests/test-readtokens.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+. "${srcdir=.}/init.sh"; path_prepend_ .
+
+fail=0
+
+test-readtokens || fail=1
+
+# Simplest case.
+echo a:b:c: > exp || fail=1
+printf a:b:c | test-readtokens : > out 2>&1 || fail=1
+compare exp out || fail=1
+
+# Use NUL as the delimiter.
+echo a:b:c: > exp || fail=1
+printf 'a\0b\0c' | test-readtokens '\0' > out 2>&1 || fail=1
+compare exp out || fail=1
+
+# Two delimiter bytes, and adjacent delimiters in the input.
+echo a:b:c: > exp || fail=1
+printf a:-:b-:c:: | test-readtokens :- > out 2>&1 || fail=1
+compare exp out || fail=1
+
+Exit $fail
--
1.7.9.3.363.g121f0