On Mon, Jul 16, 2007 at 09:55:47PM +0200, Michael Geng wrote:
> On Mon, Jul 16, 2007 at 10:02:13AM +0200, Jim Meyering wrote:
> > [EMAIL PROTECTED] (Michael Geng) wrote:
> > > I released version 0.6.6 of genparse which supports
> > > internationalization now. A link to the download page
> > > and to the updated documentation can be accessed from
> > > the genparse project page http://genparse.sourceforge.net/.
> > >
> > > I also prepared a patch which demonstrates how genparse
> > > could be used for parsing the command line arguments of
> > > the wc command. The patch is relative to the coreutils version
> > > 6.9. In order to compile the coreutils after you applied
> > > the patch you will need genparse version 0.6.6 in your path.
> > >
> > > Does this approach appear reasonable to you? What else should
> > > be improved on genparse?
> >
> > I took a quick look only at the patch (not at the code genparse generates)
> > and spotted some minor problems:
> >
> > > + if (cmdline.version)
> > > + {
> > > + version_etc (stdout, program_name, GNU_PACKAGE, VERSION,
> > AUTHORS,
> >
> > Here, it should be PROGRAM_NAME, not program_name.
I changed it to PROGRAM_NAME. Thanks.
> > > diff -u -N -r coreutils-6.9.orig/src/wc.gp coreutils-6.9/src/wc.gp
> > > --- coreutils-6.9.orig/src/wc.gp 1970-01-01 01:00:00.000000000
> > +0100
> > > +++ coreutils-6.9/src/wc.gp 2007-07-10 18:17:33.000000000 +0200
> > > @@ -0,0 +1,24 @@
> > ...
> > > +Report bugs to <[EMAIL PROTECTED]>.
> > > +#usage_end
> >
> > s/fileutils/coreutils/
> > or, perhaps better: PACKAGE_BUGREPORT
Again I released a new version of genparse (0.6.7) which now allows to include
string macros into the usage() function. So genparse is now able to use the
PACKAGE_BUGREPORT macro.
> > Did you ensure that wc still passes "make check"?
>
> I did this now - and I found a bug: wc -l doesn't work because I didn't assign
> the print_lines variable. That's not a bug in genparse but in my hand written
> code in wc.c. But after fixing this make check still fails because the usage()
> function doesn't print a proper PACKAGE_BUGREPORT as you also found.
I 'm attaching an updated patch which passes make check. You need genpare
version
0.6.7 in your path in order to compile coreutils after you applied the patch.
> > Make sure valgrind wc ... shows no leaks, too.
> > In particular, test that with the --files0-from=F option.
valgrind wc --files0-from=F tells me something about 3 not-freed blocks.
But it gives exactly the same message if I'm executing valgrind wc
--files0-from=F
from a coreutils build from the original sources of version 6.9. Is there a
known memory leak with the --files0-from=F option of wc?
I tried vagrind wc ... with some other options but it didn't report any errors
except for the --files0-from option.
> > Is there any significant difference in performance (I doubt it) or binary
> > size?
I inspected the wc which was generated using genparse with the original one and
found that the .text segment increased by 16 bytes. BTW, how can I build the
coreutils without debug information? configure CFLAGS="-O2 -g0" doesn't
remove debug information completely. Would you build coreutils differently for
comparing binary file sizes? How exactly would you compare binary file sizes?
I think comparing the .text segment size is not enough.
> > I suggest you also adapt a program that takes integer arguments.
> > Can genparse handle arguments of all different types, from int
> > through uintmax_t?
Presently genparse is able to handle int, float, char, string or flag types.
No unsigned int for example. That's another extension which I will have to
add.
> >
> > tail looks like a good candidate. It also had an optional argument:
> > --follow[={name|descriptor}]
> >
> > There are quite a few programs in the coreutils that have unusual
> > argument-parsing constraints, so it's hard to point to a small subset
> > and say "if you adapt just these few", then that should be proof enough.
> > I'm afraid that if you're interested in having coreutils convert, you'll
> > have to do most of the work, and then demonstrate that there's no
> > resulting regression (and presumably cleaner code).
> >
> > For now, let's start with tail, and maybe "ls" if you're ambitious :)
That's probably a good path to move on, but now that I started with the wc
command I wanted to get this one working first.
> > Another detail:
> > IMHO, the contents of your wc.gp file should reside in wc.c.
> > Might as well leave it in a delimited comment in place of the usage
> > function you're eliminating, then have a makefile rule use sed or awk
> > to extract it into wc.gp.
Could be done this way.
Michael
diff -u -N -r coreutils-6.9.orig/src/Makefile.am coreutils-6.9/src/Makefile.am
--- coreutils-6.9.orig/src/Makefile.am 2007-03-20 08:24:27.000000000 +0100
+++ coreutils-6.9/src/Makefile.am 2007-07-10 18:17:23.000000000 +0200
@@ -363,3 +363,7 @@
| grep -Ev -f $$t && \
{ echo 'the above variables should have static scope' 1>&2; \
exit 1; } || :
+
+wc_SOURCES = wc_clp.c wc.c
+wc_clp.c: wc.gp
+ genparse --longmembers --internationalize -o wc_clp wc.gp
diff -u -N -r coreutils-6.9.orig/src/wc.c coreutils-6.9/src/wc.c
--- coreutils-6.9.orig/src/wc.c 2007-03-18 22:36:43.000000000 +0100
+++ coreutils-6.9/src/wc.c 2007-07-16 19:40:21.000000000 +0200
@@ -24,13 +24,13 @@
#include <getopt.h>
#include <sys/types.h>
-#include "system.h"
#include "error.h"
#include "inttostr.h"
#include "quote.h"
#include "readtokens0.h"
#include "safe-read.h"
#include "wcwidth.h"
+#include "wc_clp.h"
#if !defined iswspace && !HAVE_ISWSPACE
# define iswspace(wc) \
@@ -77,60 +77,6 @@
struct stat st;
};
-/* For long options that have no equivalent short option, use a
- non-character as a pseudo short option, starting with CHAR_MAX + 1. */
-enum
-{
- FILES0_FROM_OPTION = CHAR_MAX + 1
-};
-
-static struct option const longopts[] =
-{
- {"bytes", no_argument, NULL, 'c'},
- {"chars", no_argument, NULL, 'm'},
- {"lines", no_argument, NULL, 'l'},
- {"words", no_argument, NULL, 'w'},
- {"files0-from", required_argument, NULL, FILES0_FROM_OPTION},
- {"max-line-length", no_argument, NULL, 'L'},
- {GETOPT_HELP_OPTION_DECL},
- {GETOPT_VERSION_OPTION_DECL},
- {NULL, 0, NULL, 0}
-};
-
-void
-usage (int status)
-{
- if (status != EXIT_SUCCESS)
- fprintf (stderr, _("Try `%s --help' for more information.\n"),
- program_name);
- else
- {
- printf (_("\
-Usage: %s [OPTION]... [FILE]...\n\
- or: %s [OPTION]... --files0-from=F\n\
-"),
- program_name, program_name);
- fputs (_("\
-Print newline, word, and byte counts for each FILE, and a total line if\n\
-more than one FILE is specified. With no FILE, or when FILE is -,\n\
-read standard input.\n\
- -c, --bytes print the byte counts\n\
- -m, --chars print the character counts\n\
- -l, --lines print the newline counts\n\
-"), stdout);
- fputs (_("\
- --files0-from=F read input from the files specified by\n\
- NUL-terminated names in file F\n\
- -L, --max-line-length print the length of the longest line\n\
- -w, --words print the word counts\n\
-"), stdout);
- fputs (HELP_OPTION_DESCRIPTION, stdout);
- fputs (VERSION_OPTION_DESCRIPTION, stdout);
- printf (_("\nReport bugs to <%s>.\n"), PACKAGE_BUGREPORT);
- }
- exit (status);
-}
-
/* FILE is the name of the file (or NULL for standard input)
associated with the specified counters. */
static void
@@ -580,6 +526,7 @@
char *files_from = NULL;
struct fstatus *fstatus;
struct Tokens tok;
+ struct arg_t cmdline;
initialize_main (&argc, &argv);
program_name = argv[0];
@@ -589,44 +536,25 @@
atexit (close_stdout);
- print_lines = print_words = print_chars = print_bytes = false;
- print_linelength = false;
total_lines = total_words = total_chars = total_bytes = max_line_length = 0;
- while ((optc = getopt_long (argc, argv, "clLmw", longopts, NULL)) != -1)
- switch (optc)
- {
- case 'c':
- print_bytes = true;
- break;
-
- case 'm':
- print_chars = true;
- break;
-
- case 'l':
- print_lines = true;
- break;
-
- case 'w':
- print_words = true;
- break;
-
- case 'L':
- print_linelength = true;
- break;
-
- case FILES0_FROM_OPTION:
- files_from = optarg;
- break;
-
- case_GETOPT_HELP_CHAR;
-
- case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
-
- default:
- usage (EXIT_FAILURE);
- }
+ Cmdline(&cmdline, argc, argv);
+ print_bytes = cmdline.bytes;
+ print_chars = cmdline.chars;
+ print_lines = cmdline.lines;
+ print_words = cmdline.words;
+ print_linelength = cmdline.max_line_length;
+ files_from = cmdline.files0_from;
+
+ if (cmdline.help)
+ usage (EXIT_SUCCESS, program_name);
+
+ if (cmdline.version)
+ {
+ version_etc (stdout, PROGRAM_NAME, GNU_PACKAGE, VERSION, AUTHORS,
+ (char *) NULL);
+ exit (EXIT_SUCCESS);
+ }
if (! (print_lines | print_words | print_chars | print_bytes
| print_linelength))
@@ -643,7 +571,7 @@
error (0, 0, _("extra operand %s"), quote (argv[optind]));
fprintf (stderr, "%s\n",
_("File operands cannot be combined with --files0-from."));
- usage (EXIT_FAILURE);
+ usage (EXIT_FAILURE, program_name);
}
if (STREQ (files_from, "-"))
diff -u -N -r coreutils-6.9.orig/src/wc.gp coreutils-6.9/src/wc.gp
--- coreutils-6.9.orig/src/wc.gp 1970-01-01 01:00:00.000000000 +0100
+++ coreutils-6.9/src/wc.gp 2007-07-22 08:43:52.000000000 +0200
@@ -0,0 +1,24 @@
+#include <config.h>
+#include "system.h"
+
+c / bytes flag "print the byte counts"
+m / chars flag "print the character counts"
+l / lines flag "print the newline counts"
+NONE / files0-from=F string "read input from the files specified by"
+ " NUL-terminated names in file F"
+L / max-line-length flag "print the length of the longest line"
+w / words flag "print the word counts"
+NONE / help flag "display this help and exit"
+NONE / version flag "output version information and exit"
+
+#usage_begin
+Usage: __PROGRAM_NAME__ [OPTION]... [FILE]...
+ or: __PROGRAM_NAME__ [OPTION]... --files0-from=F
+Print newline, word, and byte counts for each FILE, and a total line if
+more than one FILE is specified. With no FILE, or when FILE is -,
+read standard input.
+
+__GLOSSARY_GNU__(25)
+
+Report bugs to <__STRING__(PACKAGE_BUGREPORT)>.
+#usage_end
_______________________________________________
Bug-coreutils mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-coreutils
