Re: [PATCH] Making use of argp in GRUB utilities

[Lubomir Kundrak <lkundrak@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

Forgot to attache the file.
Here is it.

On Št, 2007-01-25 at 12:25 +0100, Lubomir Kundrak wrote:
> 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,
-- 
Lubomir Kundrak (Red Hat Security Response Team)

[-- Attachment #2: grub2-argp-grub-mkimage.patch --]
[-- Type: text/x-patch, Size: 4463 bytes --]

Index: util/i386/pc/grub-mkimage.c
===================================================================
RCS file: /sources/grub/grub2/util/i386/pc/grub-mkimage.c,v
retrieving revision 1.11
diff -u -p -r1.11 grub-mkimage.c
--- util/i386/pc/grub-mkimage.c	25 Nov 2006 03:21:29 -0000	1.11
+++ util/i386/pc/grub-mkimage.c	25 Jan 2007 11:18:41 -0000
@@ -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
 
 \f
 
-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};
+\f
 
 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;
 }