Re: [PATCH] better diagnostics for seq

[Steven Schubiger]

Jim Meyering <address@hidden> wrote:
> Thanks for working on that.

Attached is a revised patch that should take "appropriately" care of
your suggestions. I ran make check and all tests passed. Furthermore,
I checked coreutils.texi, but there seems to be no relevant documentation
for seq with regard to diagnostics (as expected). FYI, make distcheck
ungracefully exits with "fuzzy patch".

Steven Schubiger

diff --git a/ChangeLog-2008 b/ChangeLog-2008
index aac9feb..df88058 100644
--- a/ChangeLog-2008
+++ b/ChangeLog-2008
@@ -1,3 +1,13 @@
+2008-02-18  Steven Schubiger  <address@hidden>
+
+       seq: give better diagnostics for invalid formats.
+       * src/seq.c: (validate_format): New function.
+       (main): Use it.
+       * tests/misc/seq: Test for expected diagnostics with
+       invalid formats.
+       * NEWS: Mention this change.
+       * TODO: Remove this item.
+
 2008-02-07  Jim Meyering  <address@hidden>
 
        We *do* need two different version files.
diff --git a/NEWS b/NEWS
index 34fda4e..6cd73d4 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,8 @@ GNU coreutils NEWS                                    -*- 
outline -*-
 
   ls --color no longer outputs unnecessary escape sequences
 
+  seq gives better diagnostics for invalid formats.
+
 ** Consistency
 
   mkdir and split now write --verbose output to stdout, not stderr.
diff --git a/TODO b/TODO
index 8c6b6fc..3f4d26c 100644
--- a/TODO
+++ b/TODO
@@ -61,8 +61,6 @@ printf: consider adapting builtins/printf.def from bash
 
 df: add `--total' option, suggested here http://bugs.debian.org/186007
 
-seq: give better diagnostics for invalid formats:
-   e.g. no or too many % directives
 seq: consider allowing format string to contain no %-directives
 
 tail: don't use xlseek; it *exits*.
diff --git a/src/seq.c b/src/seq.c
index 261a44b..08fb664 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -1,5 +1,5 @@
 /* seq - print sequence of numbers to standard output.
-   Copyright (C) 1994-2007 Free Software Foundation, Inc.
+   Copyright (C) 1994-2008 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
@@ -177,6 +177,35 @@ scan_arg (const char *arg)
   return ret;
 }
 
+/* Validate the FORMAT format. Print a diagnostic and exit
+   when no directives or too many were found.  */
+
+static void
+validate_format (char const *fmt)
+{
+  unsigned int n_directives = 0;
+  char const *p;
+
+  for (p = fmt; *p; p++)
+    {
+      if (*p == '%')
+        {
+          if (p[1] != '%' && p[1] != '\0')
+            {
+              ++n_directives;
+              ++p;
+            }
+        }
+    }
+  if (! n_directives)
+    {
+      error (0, 0, _("no %% directive in format %s"), quote (fmt));
+      usage (EXIT_FAILURE);
+    }
+  else if (n_directives > 1)
+    error (EXIT_FAILURE, 0, _("too many %% directives in format %s"), quote 
(fmt));
+}
+
 /* If FORMAT is a valid printf format for a double argument, return
    its long double equivalent, possibly allocated from dynamic
    storage, and store into *LAYOUT a description of the output layout;
@@ -405,7 +434,11 @@ main (int argc, char **argv)
 
   if (format_str)
     {
-      char const *f = long_double_format (format_str, &layout);
+      char const *f;
+
+      validate_format (format_str);
+
+      f = long_double_format (format_str, &layout);
       if (! f)
        {
          error (0, 0, _("invalid format string: %s"), quote (format_str));
diff --git a/tests/misc/seq b/tests/misc/seq
index 9c1e48f..4e2d128 100755
--- a/tests/misc/seq
+++ b/tests/misc/seq
@@ -2,7 +2,7 @@
 # -*- perl -*-
 # Test "seq".
 
-# Copyright (C) 1999, 2000, 2003, 2005-2007 Free Software Foundation, Inc.
+# Copyright (C) 1999, 2000, 2003, 2005-2008 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
@@ -82,6 +82,13 @@ my @Tests =
    ['fmt-c',   qw(-f %%g 1), {EXIT => 1},
     {ERR => "seq: invalid format string: `%%g'\n"
      . "Try `seq --help' for more information.\n"},
+   ],
+   ['fmt-d',   qw(-f "" 1), {EXIT => 1},
+    {ERR => "seq: no % directive in format `'\n"
+     . "Try `seq --help' for more information.\n"},
+   ],
+   ['fmt-e',   qw(-f %g%g 1), {EXIT => 1},
+    {ERR => "seq: too many % directives in format `%g%g'\n"},
 ],
   );