[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: xstrtol.h vs. gnulib-tool --pobase
|
From: |
Paul Eggert |
|
Subject: |
Re: xstrtol.h vs. gnulib-tool --pobase |
|
Date: |
Wed, 08 Aug 2007 15:16:22 -0700 |
|
User-agent: |
Gnus/5.11 (Gnus v5.11) Emacs/22.1 (gnu/linux) |
By chance I was working in the same area. I merged my changes into
yours and came up with the following proposal. It's in two parts, the
first for gnulib and the second for coreutils (both in vc-dwim
format). Basically, the idea is to drop the macro STRTOL_FATAL_ERROR
and substitute a function for it, such that the caller doesn't need to
do memory allocation. This also removes some of the OPT_STR_INIT
hacks I contributed to coreutils last week.
===== gnulib changes =====
* NEWS: In xstrtol, remove STRTOL_FATAL_ERROR and add xstrtol_fatal.
* lib/xstrtol.h: Don't include exitfail.h; that's now internal to
xstrtol.c. Include getopt.h, since xstrtol_fatal's signature
depends on it.
(xstrtol_error): Remove.
(xstrtol_fatal): New decl, replacing the functionality of xstrtol_error
but with a different signature.
(ATTRIBUTE_NORETURN, __attribute__): New macros.
* lib/xstrtol-error.c: Include exitfail.h.
(xstrtol_fatal): New function, with a different signature from the
old xstrtol_error, so that the caller need not worry about passing
in an exit status, or about storage management of the option argument.
(xstrtol_error): Now a static function. Redo signature to
implement xstrtol_fatal. Output the correct number of hyphens in
front of the option so that the caller need not worry about
storage management.
(N_): New macro.
(_): Remove; not used now.
* modules/xstrtol: Depend on getopt.
* tests/test-xstrtol.c (main): Use new xstrtol_error function instead
of old STRTOL_FATAL_ERROR macro.
* tests/test-xstrtol.sh (t-xstrtol.xo): Adjust to match new behavior
of test program.
Index: NEWS
===================================================================
RCS file: /cvsroot/gnulib/gnulib/NEWS,v
retrieving revision 1.27
diff -u -p -r1.27 NEWS
--- NEWS 6 Aug 2007 16:44:25 -0000 1.27
+++ NEWS 8 Aug 2007 22:11:32 -0000
@@ -6,6 +6,9 @@ User visible incompatible changes
Date Modules Changes
+2007-08-08 xstrtol STRTOL_FATAL_ERROR is removed.
+ Use the new xstrtol_fatal function instead.
+
2007-08-04 human The function human_options no longer reports an
error to standard error; that is now the
caller's responsibility. It returns an
Index: lib/xstrtol.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/xstrtol.h,v
retrieving revision 1.26
diff -u -p -r1.26 xstrtol.h
--- lib/xstrtol.h 8 Aug 2007 12:45:54 -0000 1.26
+++ lib/xstrtol.h 8 Aug 2007 22:11:32 -0000
@@ -20,8 +20,7 @@
#ifndef XSTRTOL_H_
# define XSTRTOL_H_ 1
-# include "exitfail.h"
-
+# include <getopt.h>
# include <inttypes.h>
# ifndef _STRTOL_ERROR
@@ -48,16 +47,33 @@ _DECLARE_XSTRTOL (xstrtoul, unsigned lon
_DECLARE_XSTRTOL (xstrtoimax, intmax_t)
_DECLARE_XSTRTOL (xstrtoumax, uintmax_t)
-/* Report an error for an out-of-range integer argument.
- EXIT_CODE is the exit code (0 for a non-fatal error).
- OPTION is the option that takes the argument
- (usually starting with one or two minus signs).
- ARG is the option's argument.
- ERR is the error code returned by one of the xstrto* functions. */
-void xstrtol_error (int exit_code, char const *option, char const *arg,
- strtol_error err);
+#ifndef __attribute__
+# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 8) || __STRICT_ANSI__
+# define __attribute__(x)
+# endif
+#endif
+
+#ifndef ATTRIBUTE_NORETURN
+# define ATTRIBUTE_NORETURN __attribute__ ((__noreturn__))
+#endif
+
+/* Report an error for an invalid integer in an option argument.
+
+ ERR is the error code returned by one of the xstrto* functions.
+
+ Use OPT_IDX to decide whether to print the short option string "C"
+ or "-C" or a long option string derived from LONG_OPTION. OPT_IDX
+ is -2 if the short option "C" was used, without any leading "-"; it
+ is -1 if the short option "-C" was used; otherwise it is an index
+ into LONG_OPTIONS, which should have a name preceded by two '-'
+ characters.
+
+ ARG is the option-argument containing the integer.
+
+ After reporting an error, exit with a failure status. */
-# define STRTOL_FATAL_ERROR(Option, Arg, Err) \
- xstrtol_error (exit_failure, Option, Arg, Err)
+void xstrtol_fatal (enum strtol_error,
+ int, char, struct option const *,
+ char const *) ATTRIBUTE_NORETURN;
#endif /* not XSTRTOL_H_ */
Index: lib/xstrtol-error.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/xstrtol-error.c,v
retrieving revision 1.2
diff -u -p -r1.2 xstrtol-error.c
--- lib/xstrtol-error.c 8 Aug 2007 16:05:58 -0000 1.2
+++ lib/xstrtol-error.c 8 Aug 2007 22:11:32 -0000
@@ -23,39 +23,77 @@
#include <stdlib.h>
#include "error.h"
+#include "exitfail.h"
#include "gettext.h"
-#define _(str) gettext (str)
+#define N_(msgid) msgid
-/* Report an error for an out-of-range integer argument.
- EXIT_CODE is the exit code (0 for a non-fatal error).
- OPTION is the option that takes the argument
- (usually starting with one or two minus signs).
- ARG is the option's argument.
- ERR is the error code returned by one of the xstrto* functions. */
-void
-xstrtol_error (int exit_code, char const *option, char const *arg,
- strtol_error err)
+/* Report an error for an invalid integer in an option argument.
+
+ ERR is the error code returned by one of the xstrto* functions.
+
+ Use OPT_IDX to decide whether to print the short option string "C"
+ or "-C" or a long option string derived from LONG_OPTION. OPT_IDX
+ is -2 if the short option "C" was used, without any leading "-"; it
+ is -1 if the short option "-C" was used; otherwise it is an index
+ into LONG_OPTIONS, which should have a name preceded by two '-'
+ characters.
+
+ ARG is the option-argument containing the integer.
+
+ After reporting an error, exit with status EXIT_STATUS if it is
+ nonzero. */
+
+static void
+xstrtol_error (enum strtol_error err,
+ int opt_idx, char c, struct option const *long_options,
+ char const *arg,
+ int exit_status)
{
+ char const *hyphens = "--";
+ char const *msgid;
+ char const *option;
+ char option_buffer[2];
+
switch (err)
{
default:
abort ();
case LONGINT_INVALID:
- error (exit_code, 0, _("invalid %s argument `%s'"),
- option, arg);
+ msgid = N_("invalid %s%s argument `%s'");
break;
case LONGINT_INVALID_SUFFIX_CHAR:
- case LONGINT_INVALID_SUFFIX_CHAR | LONGINT_OVERFLOW:
- error (exit_code, 0, _("invalid suffix in %s argument `%s'"),
- option, arg);
+ case LONGINT_INVALID_SUFFIX_CHAR_WITH_OVERFLOW:
+ msgid = N_("invalid suffix in %s%s argument `%s'");
break;
case LONGINT_OVERFLOW:
- error (exit_code, 0, _("%s argument `%s' too large"),
- option, arg);
+ msgid = N_("%s%s argument `%s' too large");
break;
}
+
+ if (opt_idx < 0)
+ {
+ hyphens -= opt_idx;
+ option_buffer[0] = c;
+ option_buffer[1] = '\0';
+ option = option_buffer;
+ }
+ else
+ option = long_options[opt_idx].name;
+
+ error (exit_failure, 0, gettext (msgid), hyphens, option, arg);
+}
+
+/* Like xstrtol_error, except exit with a failure status. */
+
+void
+xstrtol_fatal (enum strtol_error err,
+ int opt_idx, char c, struct option const *long_options,
+ char const *arg)
+{
+ xstrtol_error (err, opt_idx, c, long_options, arg, exit_failure);
+ abort ();
}
Index: modules/xstrtol
===================================================================
RCS file: /cvsroot/gnulib/gnulib/modules/xstrtol,v
retrieving revision 1.17
diff -u -p -r1.17 xstrtol
--- modules/xstrtol 8 Aug 2007 12:45:55 -0000 1.17
+++ modules/xstrtol 8 Aug 2007 22:11:32 -0000
@@ -11,6 +11,7 @@ m4/xstrtol.m4
Depends-on:
exitfail
error
+getopt
gettext-h
intprops
inttypes
Index: tests/test-xstrtol.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/tests/test-xstrtol.c,v
retrieving revision 1.1
diff -u -p -r1.1 test-xstrtol.c
--- tests/test-xstrtol.c 8 Aug 2007 12:45:56 -0000 1.1
+++ tests/test-xstrtol.c 8 Aug 2007 22:11:32 -0000
@@ -52,7 +52,7 @@ main (int argc, char **argv)
}
else
{
- STRTOL_FATAL_ERROR ("arg", argv[i], s_err);
+ xstrtol_error (s_err, -2, '!', NULL, argv[i]);
}
}
exit (0);
Index: tests/test-xstrtol.sh
===================================================================
RCS file: /cvsroot/gnulib/gnulib/tests/test-xstrtol.sh,v
retrieving revision 1.1
diff -u -p -r1.1 test-xstrtol.sh
--- tests/test-xstrtol.sh 8 Aug 2007 12:45:56 -0000 1.1
+++ tests/test-xstrtol.sh 8 Aug 2007 22:11:32 -0000
@@ -40,19 +40,19 @@ cat > t-xstrtol.xo <<EOF
1->1 ()
-1->-1 ()
1k->1024 ()
-invalid suffix in arg argument \`${too_big}h'
-arg argument \`$too_big' too large
-invalid arg argument \`x'
-invalid suffix in arg argument \`9x'
+invalid suffix in ! argument \`${too_big}h'
+! argument \`$too_big' too large
+invalid ! argument \`x'
+invalid suffix in ! argument \`9x'
010->8 ()
MiB->1048576 ()
1->1 ()
-invalid arg argument \`-1'
+invalid ! argument \`-1'
1k->1024 ()
-invalid suffix in arg argument \`${too_big}h'
-arg argument \`$too_big' too large
-invalid arg argument \`x'
-invalid suffix in arg argument \`9x'
+invalid suffix in ! argument \`${too_big}h'
+! argument \`$too_big' too large
+invalid ! argument \`x'
+invalid suffix in ! argument \`9x'
010->8 ()
MiB->1048576 ()
EOF
===== coreutils changes =====
* src/df.c (long_options): Don't bother prepending "--" to long
options that OPT_STR might decode, as that hack is no longer needed.
(main): Invoke xstrtol_fatal rather than STRTOL_FATAL_ERROR.
* src/du.c (long_options, main): Likewise.
* src/ls.c (decode_switches): Likewise.
* src/od.c (long_options, main): Likewise.
* src/pr.c (first_last_page, main): Likewise.
* src/sort.c (long_options, specify_sort_size): Likewise.
* src/pr.c (first_last_page): Accept option index and option char
instead of an assembled option string. All callers changed.
* src/sort.c (specify_sort_size): Likewise.
* src/system.h (OPT_STR, LONG_OPT_STR, short_opt_str, OPT_STR_INIT):
Remove.
diff --git a/src/df.c b/src/df.c
index c2d1180..94cf330 100644
--- a/src/df.c
+++ b/src/df.c
@@ -121,7 +121,7 @@ enum
static struct option const long_options[] =
{
{"all", no_argument, NULL, 'a'},
- {OPT_STR_INIT ("block-size"), required_argument, NULL, 'B'},
+ {"block-size", required_argument, NULL, 'B'},
{"inodes", no_argument, NULL, 'i'},
{"human-readable", no_argument, NULL, 'h'},
{"si", no_argument, NULL, 'H'},
@@ -815,7 +815,7 @@ main (int argc, char **argv)
enum strtol_error e = human_options (optarg, &human_output_opts,
&output_block_size);
if (e != LONGINT_OK)
- STRTOL_FATAL_ERROR (OPT_STR (oi, c, long_options), optarg, e);
+ xstrtol_fatal (e, oi, c, long_options, optarg);
}
break;
case 'i':
diff --git a/src/du.c b/src/du.c
index caacbb0..6681079 100644
--- a/src/du.c
+++ b/src/du.c
@@ -204,7 +204,7 @@ static struct option const long_options[] =
{
{"all", no_argument, NULL, 'a'},
{"apparent-size", no_argument, NULL, APPARENT_SIZE_OPTION},
- {OPT_STR_INIT ("block-size"), required_argument, NULL, 'B'},
+ {"block-size", required_argument, NULL, 'B'},
{"bytes", no_argument, NULL, 'b'},
{"count-links", no_argument, NULL, 'l'},
{"dereference", no_argument, NULL, 'L'},
@@ -796,7 +796,7 @@ main (int argc, char **argv)
enum strtol_error e = human_options (optarg, &human_output_opts,
&output_block_size);
if (e != LONGINT_OK)
- STRTOL_FATAL_ERROR (OPT_STR (oi, c, long_options), optarg, e);
+ xstrtol_fatal (e, oi, c, long_options, optarg);
}
break;
diff --git a/src/ls.c b/src/ls.c
index 064a51f..2167a4d 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1506,10 +1506,15 @@ decode_switches (int argc, char **argv)
}
}
- while ((c = getopt_long (argc, argv,
- "abcdfghiklmnopqrstuvw:xABCDFGHI:LNQRST:UXZ1",
- long_options, NULL)) != -1)
+ for (;;)
{
+ int oi = -1;
+ int c = getopt_long (argc, argv,
+ "abcdfghiklmnopqrstuvw:xABCDFGHI:LNQRST:UXZ1",
+ long_options, &oi);
+ if (c == -1)
+ break;
+
switch (c)
{
case 'a':
@@ -1797,7 +1802,7 @@ decode_switches (int argc, char **argv)
enum strtol_error e = human_options (optarg, &human_output_opts,
&output_block_size);
if (e != LONGINT_OK)
- STRTOL_FATAL_ERROR ("--block-size", optarg, e);
+ xstrtol_fatal (e, oi, 0, long_options, optarg);
file_output_block_size = output_block_size;
}
break;
diff --git a/src/od.c b/src/od.c
index 472c513..1e77f92 100644
--- a/src/od.c
+++ b/src/od.c
@@ -281,14 +281,14 @@ enum
static struct option const long_options[] =
{
- {OPT_STR_INIT ("skip-bytes"), required_argument, NULL, 'j'},
+ {"skip-bytes", required_argument, NULL, 'j'},
{"address-radix", required_argument, NULL, 'A'},
- {OPT_STR_INIT ("read-bytes"), required_argument, NULL, 'N'},
+ {"read-bytes", required_argument, NULL, 'N'},
{"format", required_argument, NULL, 't'},
{"output-duplicates", no_argument, NULL, 'v'},
- {OPT_STR_INIT ("strings"), optional_argument, NULL, 'S'},
+ {"strings", optional_argument, NULL, 'S'},
{"traditional", no_argument, NULL, TRADITIONAL_OPTION},
- {OPT_STR_INIT ("width"), optional_argument, NULL, 'w'},
+ {"width", optional_argument, NULL, 'w'},
{GETOPT_HELP_OPTION_DECL},
{GETOPT_VERSION_OPTION_DECL},
@@ -1655,7 +1655,7 @@ it must be one character from [doxn]"),
modern = true;
s_err = xstrtoumax (optarg, NULL, 0, &n_bytes_to_skip, multipliers);
if (s_err != LONGINT_OK)
- STRTOL_FATAL_ERROR (OPT_STR (oi, c, long_options), optarg, s_err);
+ xstrtol_fatal (s_err, oi, c, long_options, optarg);
break;
case 'N':
@@ -1665,7 +1665,7 @@ it must be one character from [doxn]"),
s_err = xstrtoumax (optarg, NULL, 0, &max_bytes_to_format,
multipliers);
if (s_err != LONGINT_OK)
- STRTOL_FATAL_ERROR (OPT_STR (oi, c, long_options), optarg, s_err);
+ xstrtol_fatal (s_err, oi, c, long_options, optarg);
break;
case 'S':
@@ -1676,8 +1676,7 @@ it must be one character from [doxn]"),
{
s_err = xstrtoumax (optarg, NULL, 0, &tmp, multipliers);
if (s_err != LONGINT_OK)
- STRTOL_FATAL_ERROR (OPT_STR (oi, c, long_options), optarg,
- s_err);
+ xstrtol_fatal (s_err, oi, c, long_options, optarg);
/* The minimum string length may be no larger than SIZE_MAX,
since we may allocate a buffer of this size. */
@@ -1749,8 +1748,7 @@ it must be one character from [doxn]"),
uintmax_t w_tmp;
s_err = xstrtoumax (optarg, NULL, 10, &w_tmp, "");
if (s_err != LONGINT_OK)
- STRTOL_FATAL_ERROR (OPT_STR (oi, c, long_options), optarg,
- s_err);
+ xstrtol_fatal (s_err, oi, c, long_options, optarg);
if (SIZE_MAX < w_tmp)
error (EXIT_FAILURE, 0, _("%s is too large"), optarg);
desired_width = w_tmp;
diff --git a/src/pr.c b/src/pr.c
index 329183b..1ed79d6 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -796,14 +796,14 @@ cols_ready_to_print (void)
using option +FIRST_PAGE:LAST_PAGE */
static bool
-first_last_page (char const *option, char const *pages)
+first_last_page (int oi, char c, char const *pages)
{
char *p;
uintmax_t first;
uintmax_t last = UINTMAX_MAX;
strtol_error err = xstrtoumax (pages, &p, 10, &first, "");
if (err != LONGINT_OK && err != LONGINT_INVALID_SUFFIX_CHAR)
- STRTOL_FATAL_ERROR (option, pages, err);
+ xstrtol_fatal (err, oi, c, long_options, pages);
if (p == pages || !first)
return false;
@@ -813,7 +813,7 @@ first_last_page (char const *option, char const *pages)
char const *p1 = p + 1;
err = xstrtoumax (p1, &p, 10, &last, "");
if (err != LONGINT_OK)
- STRTOL_FATAL_ERROR (option, pages, err);
+ xstrtol_fatal (err, oi, c, long_options, pages);
if (p1 == p || last < first)
return false;
}
@@ -856,7 +856,6 @@ separator_string (const char *optarg_S)
int
main (int argc, char **argv)
{
- int c;
int n_files;
bool old_options = false;
bool old_w = false;
@@ -881,9 +880,13 @@ main (int argc, char **argv)
? xmalloc ((argc - 1) * sizeof (char *))
: NULL);
- while ((c = getopt_long (argc, argv, short_options, long_options, NULL))
- != -1)
+ for (;;)
{
+ int oi = -1;
+ int c = getopt_long (argc, argv, short_options, long_options, &oi);
+ if (c == -1)
+ break;
+
if (ISDIGIT (c))
{
/* Accumulate column-count digits specified via old-style options. */
@@ -902,7 +905,7 @@ main (int argc, char **argv)
case 1: /* Non-option argument. */
/* long option --page dominates old `+FIRST_PAGE ...'. */
if (! (first_page_number == 0
- && *optarg == '+' && first_last_page ("+", optarg + 1)))
+ && *optarg == '+' && first_last_page (-2, '+', optarg + 1)))
file_names[n_files++] = optarg;
break;
@@ -911,7 +914,7 @@ main (int argc, char **argv)
if (! optarg)
error (EXIT_FAILURE, 0,
_("`--pages=FIRST_PAGE[:LAST_PAGE]' missing argument"));
- else if (! first_last_page ("--pages", optarg))
+ else if (! first_last_page (oi, 0, optarg))
error (EXIT_FAILURE, 0, _("Invalid page range %s"),
quote (optarg));
break;
diff --git a/src/sort.c b/src/sort.c
index dc7874a..620a4bc 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -414,7 +414,7 @@ static struct option const long_options[] =
{"output", required_argument, NULL, 'o'},
{"reverse", no_argument, NULL, 'r'},
{"stable", no_argument, NULL, 's'},
- {OPT_STR_INIT ("buffer-size"), required_argument, NULL, 'S'},
+ {"buffer-size", required_argument, NULL, 'S'},
{"field-separator", required_argument, NULL, 't'},
{"temporary-directory", required_argument, NULL, 'T'},
{"unique", no_argument, NULL, 'u'},
@@ -1032,7 +1032,7 @@ inittables (void)
/* Specify the amount of main memory to use when sorting. */
static void
-specify_sort_size (char const *option, char const *s)
+specify_sort_size (int oi, char c, char const *s)
{
uintmax_t n;
char *suffix;
@@ -1088,7 +1088,7 @@ specify_sort_size (char const *option, char const *s)
e = LONGINT_OVERFLOW;
}
- STRTOL_FATAL_ERROR (option, s, e);
+ xstrtol_fatal (e, oi, c, long_options, s);
}
/* Return the default sort size. */
@@ -3012,7 +3012,7 @@ main (int argc, char **argv)
break;
case 'S':
- specify_sort_size (OPT_STR (oi, c, long_options), optarg);
+ specify_sort_size (oi, c, optarg);
break;
case 't':
diff --git a/src/system.h b/src/system.h
index dcb13ba..3c7f49d 100644
--- a/src/system.h
+++ b/src/system.h
@@ -592,28 +592,3 @@ emit_bug_reporting_address (void)
bugs (typically your translation team's web or email address). */
printf (_("\nReport bugs to <%s>.\n"), PACKAGE_BUGREPORT);
}
-
-/* Use OPT_IDX to decide whether to return either a short option
- string "-C", or a long option string derived from LONG_OPTIONS.
- OPT_IDX is -1 if the short option C was used; otherwise it is an
- index into LONG_OPTIONS, which should have a name preceded by two
- '-' characters. */
-#define OPT_STR(opt_idx, c, long_options) \
- ((opt_idx) < 0 \
- ? short_opt_str (c) \
- : LONG_OPT_STR (opt_idx, long_options))
-
-/* Likewise, but assume OPT_IDX is nonnegative. */
-#define LONG_OPT_STR(opt_idx, long_options) ((long_options)[opt_idx].name - 2)
-
-/* Given the byte, C, return the string "-C" in static storage. */
-static inline char *
-short_opt_str (char c)
-{
- static char opt_str_storage[3] = {'-', 0, 0};
- opt_str_storage[1] = c;
- return opt_str_storage;
-}
-
-/* Define an option string that will be used with OPT_STR or LONG_OPT_STR. */
-#define OPT_STR_INIT(name) ("--" name + 2)
M ChangeLog
M src/df.c
M src/du.c
M src/ls.c
M src/od.c
M src/pr.c
M src/sort.c
M src/system.h
Committed as 342397b487abea6818371f7a55d7f99a41f0772e