[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] better diagnostics for seq
From: |
Jim Meyering |
Subject: |
Re: [PATCH] better diagnostics for seq |
Date: |
Tue, 19 Feb 2008 00:03:14 +0100 |
Steven Schubiger <address@hidden> wrote:
> 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.
Thanks.
BTW, I still do require ChangeLog entries, but not in any file.
Now, they end up in the git commit log, and eventually in a generated
ChangeLog that is created in the release tarball at "make dist" time.
The "fuzzy patch" error means the c99-to-c89.diff file needs
to be updated. Currently, that has to be resolved manually
by running "make patch-check REGEN_PATCH=yes" and then copying
new-diff to src/c99-to-c89.diff. I've just done that.
I made a few changes to your patch, and added a new change set
to update src/c99-to-c89.diff, below. I'll push this tomorrow.
-----------------------------------
>From f0e9eb150c4f97211de4ebd609091e2cef88898e Mon Sep 17 00:00:00 2001
From: Steven Schubiger <address@hidden>
Date: Mon, 18 Feb 2008 22:39:22 +0100
Subject: [PATCH] seq: give better diagnostics for invalid formats.
* src/seq.c: (validate_format): New function.
(main): Use it.
* tests/misc/seq (fmt-d, fmt-e): Test for expected diagnostics with
invalid formats.
* NEWS: Mention this change.
* TODO: Remove this item.
[jm: src/seq.c: make diagnostics more consistent
tests/misc/seq (fmt-eos1): adjust the expected diagnostic ]
Signed-off-by: Jim Meyering <address@hidden>
---
NEWS | 2 ++
TODO | 2 --
src/seq.c | 28 ++++++++++++++++++++++++++++
tests/misc/seq | 6 +++++-
4 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/NEWS b/NEWS
index bc1b47d..d5d6737 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,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 b073fd1..c7e8cb9 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -177,6 +177,33 @@ scan_arg (const char *arg)
return ret;
}
+/* Validate the format, FMT. Print a diagnostic and exit
+ if there is not exactly one %-directive. */
+
+static void
+validate_format (char const *fmt)
+{
+ unsigned int n_directives = 0;
+ char const *p;
+
+ for (p = fmt; *p; p++)
+ {
+ if (p[0] == '%' && p[1] != '%' && p[1] != '\0')
+ {
+ ++n_directives;
+ ++p;
+ }
+ }
+ if (! n_directives)
+ {
+ error (0, 0, _("no %% directive in format string %s"), quote (fmt));
+ usage (EXIT_FAILURE);
+ }
+ else if (1 < n_directives)
+ error (EXIT_FAILURE, 0, _("too many %% directives in format string %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,6 +432,7 @@ main (int argc, char **argv)
if (format_str)
{
+ validate_format (format_str);
char const *f = long_double_format (format_str, &layout);
if (! f)
{
diff --git a/tests/misc/seq b/tests/misc/seq
index 1a153a3..f48289b 100755
--- a/tests/misc/seq
+++ b/tests/misc/seq
@@ -87,10 +87,14 @@ my @Tests =
# "seq: memory exhausted". In coreutils-6.0..6.8, it would mistakenly
# succeed and print a blank line.
['fmt-eos1', qw(-f % 1), {EXIT => 1},
- {ERR => "seq: invalid format string: `%'\n" . $try_help }],
+ {ERR => "seq: no % directive in format string `%'\n" . $try_help }],
['fmt-eos2', qw(-f %g% 1), {EXIT => 1},
{ERR => "seq: invalid format string: `%g%'\n" . $try_help }],
+ ['fmt-d', qw(-f "" 1), {EXIT => 1},
+ {ERR => "seq: no % directive in format string `'\n" . $try_help }],
+ ['fmt-e', qw(-f %g%g 1), {EXIT => 1},
+ {ERR => "seq: too many % directives in format string `%g%g'\n"}],
);
# Append a newline to each entry in the OUT array.
--
1.5.4.2.124.gc4db3
>From 8ebdb95fb939e9710e985c504b62afa4a05c1e63 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 18 Feb 2008 23:52:26 +0100
Subject: [PATCH] * src/c99-to-c89.diff: Adjust seq.c offsets. Accommodate a
new C99-ism.
Signed-off-by: Jim Meyering <address@hidden>
---
src/c99-to-c89.diff | 22 ++++++++++++++++++++--
1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/c99-to-c89.diff b/src/c99-to-c89.diff
index 9dfa1e8..79ac4e6 100644
--- a/src/c99-to-c89.diff
+++ b/src/c99-to-c89.diff
@@ -108,7 +108,7 @@ diff -upr src/seq.c src/seq.c
}
return ret;
-@@ -311,6 +313,7 @@ get_default_format (operand first, opera
+@@ -338,6 +340,7 @@ get_default_format (operand first, opera
size_t last_width = last.width + (prec - last.precision);
if (last.precision && prec == 0)
last_width--; /* don't include space for '.' */
@@ -116,7 +116,7 @@ diff -upr src/seq.c src/seq.c
size_t width = MAX (first_width, last_width);
if (width <= INT_MAX)
{
-@@ -318,6 +321,7 @@ get_default_format (operand first, opera
+@@ -345,6 +348,7 @@ get_default_format (operand first, opera
sprintf (format_buf, "%%0%d.%dLf", w, prec);
return format_buf;
}
@@ -124,6 +124,22 @@ diff -upr src/seq.c src/seq.c
}
else
{
+@@ -433,6 +437,7 @@ main (int argc, char **argv)
+ if (format_str)
+ {
+ validate_format (format_str);
++ {
+ char const *f = long_double_format (format_str, &layout);
+ if (! f)
+ {
+@@ -440,6 +445,7 @@ main (int argc, char **argv)
+ usage (EXIT_FAILURE);
+ }
+ format_str = f;
++ }
+ }
+
+ last = scan_arg (argv[optind++]);
diff -upr src/shred.c src/shred.c
--- src/shred.c 2007-07-23 12:56:20.000000000 +0200
+++ src/shred.c 2007-07-23 13:03:12.000000000 +0200
@@ -136,3 +152,5 @@ diff -upr src/shred.c src/shred.c
if (errnum == EIO && 0 <= size && (soff | SECTOR_MASK) < lim)
{
size_t soff1 = (soff | SECTOR_MASK) + 1;
+--- src/seq.c 2008-02-18 22:53:29.000000000 +0100
++++ src-c89/seq.c 2008-02-18 23:16:35.000000000 +0100
--
1.5.4.2.124.gc4db3