Making use of argp in GRUB utilities
PATCH 1/1 Making use of argp in GRUB utilities
Hi,
I've noticed, that GRUB 2 uses argp in grub-emu [1], whereas other utilities use getopt_long [2]. Wouldn't it be nice to make this consistent?
[1] http://www.gnu.org/software/libc/manual/html_node/Argp.html
[2]
http://www.gnu.org/software/libc/manual/html_node/Getopt-Long-Options.html
I find the "GNU way," argp, approach more elegant, as demonstrated by a patch to util/i386/pc/grub-mkimage.c. Seeing a patch with more minuses than pluses is a good sign, indeed.
The main argument against agrp framework could be, that non-GNU C libraries do not contain with it. There's a standalone libargp package
[3] that is available for package systems of all major operating systems, including NetBSD and FreeBSD, so the only disadvantage is addding a dependency there.
[3] http://www.lysator.liu.se/~nisse/misc/
Regards,
Signature · 1 line
Lubomir Kundrak (Red Hat Security Response Team)
Forgot to attache the file.
Here is it.
Quoted 3 lines · Lubomir Kundrak
I find the "GNU way," argp, approach more elegant, as demonstrated by a patch to
util/i386/pc/grub-mkimage.c. Seeing a patch with more minuses than pluses is a good sign, indeed.
Regards,
Signature · 1 line
Lubomir Kundrak (Red Hat Security Response Team)
Index: util/i386/pc/grub-mkimage.c
===================================================================
RCS file: /sources/grub/grub2/util/i386/pc/grub-mkimage.c,v retrieving revision 1.11
i386/pc/grub-mkimage.c +58 -81
@@ -1,7 +1,7 @@/* grub-mkimage.c - make a bootable image */ /* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2004,2005,2006 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2004,2005,2006,2007 Free Software Foundation, Inc. * * GRUB is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -33,7 +33,7 @@#include <stdlib.h> #define _GNU_SOURCE 1 -#include <getopt.h> +#include <argp.h> #if defined(HAVE_LZO_LZO1X_H )# include <lzo/lzo1x.h>
@@ -179,102 +179,79 @@ generate_image (const char *dir, FILE *o -static struct option options[] = - { - {"directory" , required_argument,0 ,'d' }, - {"output" , required_argument,0 ,'o' }, - {"help" , no_argument,0 ,'h' }, - {"version" , no_argument,0 ,'V' }, - {"verbose" , no_argument,0 ,'v' }, - {0 ,0 ,0 ,0 } - }; +const char *argp_program_version =PACKAGE_STRING ; +const char *argp_program_bug_address =PACKAGE_BUGREPORT ; +static char doc[] ="Make a bootable image of GRUB" ; + +static struct argp_option options[] = { + {"directory" ,'d' ,"DIR" ,0 ,"use images and modules under DIR [default=" GRUB_DATADIR "]" ,0 }, + {"output" ,'o' ,"FILE" ,0 ,"output a generated image to FILE [default=stdout]" ,0 }, + {"verbose" ,'v' ,0 ,0 ,"print verbose messages" ,0 }, + {0 ,0 ,0 ,0 ,0 ,0 } +}; -static void -usage (int status) +struct arguments { -if (status) -fprintf (stderr,"Try ``grub-mkimage --help'' for more information.\n" ); -else -printf ("\ -Usage: grub-mkimage [OPTION]... [MODULES]\n\ -\n\ -Make a bootable image of GRUB.\n\ -\n\ --d, --directory=DIR use images and modules under DIR [default=%s]\n\ --o, --output=FILE output a generated image to FILE [default=stdout]\n\ --h, --help display this message and exit\n\ --V, --version print version information and exit\n\ --v, --verbose print verbose messages\n\ -\n\ -Report bugs to <%s>.\n\ -" ,GRUB_LIBDIR ,PACKAGE_BUGREPORT ); +char *dir; +char *output; +char **modules; +}; -exit (status); -} +static error_t +parse_opt (int key,char *arg,struct argp_state *state) +{ +struct arguments *args = state->input; + +switch (key) + { +case 'd' : + args->dir = arg; +break ; +case 'o' : + args->output = arg; +break ; +case 'v' : + verbosity++; +break ; +case ARGP_KEY_ARGS : + args->modules = state->argv + state->next; +break ; +case ARGP_KEY_END : +if (! args->modules) + { args->modules = state->argv + state->next; } +break ; +default : +return ARGP_ERR_UNKNOWN ; + } +return 0 ; +} + +static struct argp argp = {options, parse_opt,"[MODULE...]" , doc,0 ,0 ,0 }; +int main (int argc,char *argv[]) { -char *output =0 ; -char *dir =0 ;FILE *fp = stdout; +struct arguments args = + { + .dir =GRUB_DATADIR , + .output =0 + }; progname ="grub-mkimage" ; - -while (1 ) - { -int c =getopt_long (argc, argv,"d:o:hVv" , options,0 ); -if (c ==-1 ) -break ; -else -switch (c) - { -case 'o' : -if (output) -free (output); - - output =xstrdup (optarg); -break ; - -case 'd' : -if (dir) -free (dir); - - dir =xstrdup (optarg); -break ; - -case 'h' : -usage (0 ); -break ; - -case 'V' : -printf ("grub-mkimage (%s) %s\n" ,PACKAGE_NAME ,PACKAGE_VERSION ); -return 0 ; - -case 'v' : - verbosity++; -break ; - -default : -usage (1 ); -break ; - } - } +argp_parse (&argp, argc, argv,0 ,0 , &args); -if (output) +if (args.output) { - fp =fopen (output,"wb" ); + fp =fopen (args.output,"wb" );if (! fp) -grub_util_error ("cannot open %s" , output); +grub_util_error ("cannot open %s" , args.output); } -generate_image (dir ? :GRUB_LIBDIR , fp, argv + optind); +generate_image (args.dir ? :GRUB_LIBDIR , fp, args.modules);fclose (fp); - -if (dir) -free (dir); -return 0 ; }
Quoted 3 lines · Lubomir Kundrak
I've noticed, that GRUB 2 uses argp in grub-emu [1], whereas other utilities use
getopt_long[2]. Wouldn't it be nice to make this consistent?
Maybe.
Quoted 9 lines
I find the "GNU way," argp, approach more elegant, as demonstrated by a patch to
util/i386/pc/grub-mkimage.c. Seeing a patch with more minuses than pluses is a good sign, indeed.The main argument against agrp framework could be, that non-GNU C libraries do not contain with it. There's a standalone libargp package
[3] that is available for package systems of all major operating systems, including NetBSD and FreeBSD, so the only disadvantage is addding a dependency there.
I preferred that libargp would be included in our source tree so that it would be used when argp is not found in a system, but I guess Marco hasn't done it yet. This depends on which is more convenient for users, using an external shared library, or using our own. In GRUB Legacy, I included getopt for *BSD, and I got positive answers. So I feel that it would be better to include.
Okuji
Quoted 17 lines · Yoshinori K. Okuji
Quoted 3 lines · Lubomir Kundrak
I've noticed, that GRUB 2 uses argp in grub-emu [1], whereas other utilities use
getopt_long[2]. Wouldn't it be nice to make this consistent?Maybe.
Quoted 9 lines
I find the "GNU way," argp, approach more elegant, as demonstrated by a patch to
util/i386/pc/grub-mkimage.c. Seeing a patch with more minuses than pluses is a good sign, indeed.The main argument against agrp framework could be, that non-GNU C libraries do not contain with it. There's a standalone libargp package
[3] that is available for package systems of all major operating systems, including NetBSD and FreeBSD, so the only disadvantage is addding a dependency there.
Quoted 5 lines
I preferred that libargp would be included in our source tree so that it would be used when argp is not found in a system, but I guess Marco hasn't done it yet. This depends on which is more convenient for users, using an external shared library, or using our own. In GRUB Legacy, I included getopt for *BSD, and I got positive answers. So I feel that it would be better to include.
Sound reasonable. Would it be of any use if I converted other utils to make use of argp in my spare time?
Quoted 2 lines
Okuji
Signature · 1 line
Lubomir Kundrak (Red Hat Security Response Team)
Quoted 8 lines · Lubomir Kundrak
Quoted 5 lines
I preferred that libargp would be included in our source tree so that it would be used when argp is not found in a system, but I guess Marco hasn't done it yet. This depends on which is more convenient for users, using an external shared library, or using our own. In GRUB Legacy, I included getopt for *BSD, and I got positive answers. So I feel that it would be better to include.
Sound reasonable. Would it be of any use if I converted other utils to make use of argp in my spare time?
It seems either that Okuji doesn't care or doesn't know yet. I personally prefer argp because it is easy to use, flexible and less work to maintain. And like you said, less code... less code normally means less bugs.
So it depends on Okuji what to do. If he doesn't care I would be happy with such a patch.
--
Marco
Quoted 11 lines · Marco Gerards
Quoted 9 lines · Lubomir Kundrak
Quoted 6 lines
I preferred that libargp would be included in our source tree so that it would be used when argp is not found in a system, but I guess Marco hasn't done it yet. This depends on which is more convenient for users, using an external shared library, or using our own. In GRUB Legacy, I included getopt for *BSD, and I got positive answers. So I feel that it would be better to include.
Sound reasonable. Would it be of any use if I converted other utils to make use of argp in my spare time?
Quoted 7 lines
It seems either that Okuji doesn't care or doesn't know yet. I personally prefer argp because it is easy to use, flexible and less work to maintain. And like you said, less code... less code normally means less bugs.
So it depends on Okuji what to do. If he doesn't care I would be happy with such a patch.
I don't know. I agree that using a single library for the same goal is more consistent. However, I am so used to getopt, and I have no problem with it. Thus I am not eager to migrate to argp in particular, although I do not object to using argp.
(In my whole life, I have used argp only once for Hurd.)
Okuji
"Yoshinori K. Okuji" <[email protected]> writes:
Hi,
Quoted 17 lines · Lubomir Kundrak
Quoted 3 lines
I've noticed, that GRUB 2 uses argp in grub-emu [1], whereas other utilities use
getopt_long[2]. Wouldn't it be nice to make this consistent?Maybe.
Quoted 9 lines
I find the "GNU way," argp, approach more elegant, as demonstrated by a patch to
util/i386/pc/grub-mkimage.c. Seeing a patch with more minuses than pluses is a good sign, indeed.The main argument against agrp framework could be, that non-GNU C libraries do not contain with it. There's a standalone libargp package
[3] that is available for package systems of all major operating systems, including NetBSD and FreeBSD, so the only disadvantage is addding a dependency there.
Quoted 5 lines
I preferred that libargp would be included in our source tree so that it would be used when argp is not found in a system, but I guess Marco hasn't done it yet. This depends on which is more convenient for users, using an external shared library, or using our own. In GRUB Legacy, I included getopt for *BSD, and I got positive answers. So I feel that it would be better to include.
There is the library you mentioned and the argp implementation from gnulib. I personally prefer gnulib, because we don't have to worry about copyright at all in this case. But I agree, including it so things work on BSD is prefered.
I assume you noticed grub-emu isn't compiled by default. It makes the situation a bit easier when using BSD. Although for a good port we need some argp implementation on BSD.
--
Marco
Quoted 4 lines · Marco Gerards
There is the library you mentioned and the argp implementation from gnulib. I personally prefer gnulib, because we don't have to worry about copyright at all in this case. But I agree, including it so things work on BSD is prefered.
Yes, it is a good idea to steal code from gnulib. Gnulib exists for the very purpose.
Okuji