Steven Schubiger <[EMAIL PROTECTED]> wrote:
> Attached is a patch that enhances seq's diagnostics. If you agree
> that this is the right way to go, I'll amend other files (ChangeLog,
> etc.) as needed.
Thanks for working on that.
> diff --git a/src/seq.c b/src/seq.c
> index 261a44b..4a6f96e 100644
> --- a/src/seq.c
> +++ b/src/seq.c
> @@ -185,12 +185,38 @@ scan_arg (const char *arg)
> static char const *
> long_double_format (char const *fmt, struct layout *layout)
> {
> + const char *fmt_scan;
> size_t i;
> + size_t counted_directives = 0;
> size_t prefix_len = 0;
> size_t suffix_len = 0;
> size_t length_modifier_offset;
> bool has_L;
>
> + fmt_scan = (const char *) fmt;
> + while (*fmt_scan)
> + {
> + if (*fmt_scan == ' ')
> + fmt_scan++;
> + else
> + {
> + if (*fmt_scan == '%'
> + && (*(fmt_scan + 1) != '%'
> + && *(fmt_scan + 1) != ' '
> + && *(fmt_scan + 1) != '\0'))
> + {
> + counted_directives++;
> + fmt_scan += 2;
> + }
> + else
> + fmt_scan++;
> + }
> + }
> + if (counted_directives == 0)
> + error (EXIT_FAILURE, 0, _("no %% directive"));
This might be a little more readable:
error (EXIT_FAILURE, 0, _("no %% directive in format %s"), quote (fmt));
> + else if (counted_directives > 1)
> + error (EXIT_FAILURE, 0, _("too many %% directives"));
> +
I think you can simplify that.
How about something like this?
unsigned int n_directives = 0;
char const *p = fmt;
for (p = fmt; *p; p++)
{
if (*p == '%')
{
if (p[1] != '%' && p[1] != '\0')
{
++n_directives;
++p;
}
}
}
Of course, it'd be nice to have a comment or two.
Even better: just add a new function to validate the format string,
including your two new diagnostics. Then the standard function-
describing comment will suffice.
> - if (! strchr ("efgaEFGA", fmt[i]))
> - return NULL;
> + if (! strchr ("efgaEFGA", fmt[i]))
> + error (EXIT_FAILURE, 0, _("invalid directive: `%c%c'"), fmt[i - 1],
> fmt[i]);
It's probably not worth a new test here,
since this is already handled by the caller.
The full directive may be many bytes long, so in general, %c%c is not
enough. Also, please don't add literal `' quotes in diagnostics.
If you want to quote something, use one of the quote* functions.
Please add test cases that exercise the new code
and test for the expected (new) diagnostics.
Each new test case should be in the form of a new line or two
in the file, tests/misc/seq.
When testing for a diagnostic, use this one as a model:
['fmt-c', qw(-f %%g 1), {EXIT => 1},
{ERR => "seq: invalid format string: `%%g'\n"
. "Try `seq --help' for more information.\n"},
_______________________________________________
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
