Re: [bug-inetutils] Argpifying ifconfig.

[Sergey Poznyakoff]

Debarshi 'Rishi' Ray <address@hidden> wrote:

> As far as I understand, the traditional ifconfig invocation was being
> handled by this snippet of code after the while loop involving getopt

OK, here is the fixed patch, than. Notice `#define program_name
program_invocation_short_name' in ifconfig.h and removal of the
program_name global.  The initialization of
program_invocation_short_name is done by argp automatically, so there's
no use assigning it explicitely.

Regards,
Sergey

Index: Makefile.am
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ifconfig/Makefile.am,v
retrieving revision 1.9
diff -p -u -r1.9 Makefile.am
--- Makefile.am 12 Oct 2006 09:55:06 -0000      1.9
+++ Makefile.am 4 Apr 2007 18:37:46 -0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Free Software Foundation, Inc.
+# Copyright (C) 2005, 2007 Free Software Foundation, Inc.
 # This file is part of GNU Inetutils.
 #
 # GNU Inetutils is free software; you can redistribute it and/or modify
@@ -28,5 +28,5 @@ noinst_HEADERS = ifconfig.h if_index.h f
 
 @PATHDEFS_MAKE@
 
-INCLUDES = -I$(top_srcdir)/lib -I../lib 
+INCLUDES = -I$(top_srcdir)/libinetutils -I$(top_srcdir)/lib -I../lib 
 LDADD = -L../libinetutils -linetutils ../lib/libgnu.a
Index: ifconfig.c
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ifconfig/ifconfig.c,v
retrieving revision 1.7
diff -p -u -r1.7 ifconfig.c
--- ifconfig.c  21 Oct 2006 15:24:20 -0000      1.7
+++ ifconfig.c  4 Apr 2007 18:37:46 -0000
@@ -1,6 +1,6 @@
 /* ifconfig.c -- network interface configuration utility
 
-   Copyright (C) 2001, 2002 Free Software Foundation, Inc.
+   Copyright (C) 2001, 2002, 2007 Free Software Foundation, Inc.
 
    Written by Marcus Brinkmann.
 
@@ -60,7 +60,9 @@ main (int argc, char *argv[])
   int sfd;
   struct ifconfig *ifp;
 
-  parse_opt (argc, argv);
+  /* Parse command line */
+  if (argp_parse (&argp, argc, argv, 0, NULL, NULL))
+    exit (1);
 
   sfd = socket (AF_INET, SOCK_STREAM, 0);
   if (sfd < 0)
Index: ifconfig.h
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ifconfig/ifconfig.h,v
retrieving revision 1.5
diff -p -u -r1.5 ifconfig.h
--- ifconfig.h  21 Oct 2006 15:24:20 -0000      1.5
+++ ifconfig.h  4 Apr 2007 18:37:46 -0000
@@ -30,4 +30,6 @@
 /* XXX */
 extern int configure_if (int sfd, struct ifconfig *ifp);
 
+#define program_name program_invocation_short_name
+
 #endif
Index: options.c
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ifconfig/options.c,v
retrieving revision 1.11
diff -p -u -r1.11 options.c
--- options.c   21 Oct 2006 15:24:20 -0000      1.11
+++ options.c   4 Apr 2007 18:37:46 -0000
@@ -1,6 +1,6 @@
 /* options.c -- process the command line options
 
-   Copyright (C) 2001, 2002 Free Software Foundation, Inc.
+   Copyright (C) 2001, 2002, 2007 Free Software Foundation, Inc.
 
    Written by Marcus Brinkmann.
 
@@ -44,14 +44,54 @@
 
 #include <sys/socket.h>
 #include <net/if.h>
+#include <libinetutils.h>
 #include "ifconfig.h"
 
+const char args_doc[] = "[SYSTEM OPTION...]";
+const char doc[] = "Configure network interfaces.";
+
+/* Define keys for long options that do not have short counterparts. */
+enum {
+  ARG_BRDADDR = 256,
+  ARG_METRIC,
+  ARG_FORMAT
+};
+
+static struct argp_option argp_options[] = {
+#define GRP 0
+  {NULL, 0, NULL, 0, "Options are:", GRP},
+  {"address", 'a', "ADDR", 0, "Set interface address to ADDR", GRP+1},
+  {"interface", 'i', "NAME", 0, "Configure network interface NAME", GRP+1},
+  {"dstaddr", 'd', "DSTADDR", 0, "Set destination (peer) address to DSTADDR",
+   GRP+1},
+  {"peer", 'p', "DSTADDR", OPTION_ALIAS, "", GRP+1},
+  {"broadcast", 'b', "BRDADDR", 0, "Set broadcast address to BRDADDR", GRP+1},
+  {"brdaddr", ARG_BRDADDR, "BRDADDR", OPTION_ALIAS, "", GRP+1},
+  {"mtu", 'M', "N", 0, "Set mtu of interface to N", GRP+1},
+  {"metric", ARG_METRIC, "N", 0, "Set metric of interface to N", GRP+1},
+  {"netmask", 'm', "MASK", 0, "Set netmask to MASK", GRP+1},
+  {"format", ARG_FORMAT, "FORMAT", 0, "Select output format (or set back to "
+   "default)", GRP+1},
+  {"verbose", 'v', NULL, 0, "Output information when configuring interface.",
+   GRP+1},
+#undef GRP
+#ifdef SYSTEM_LONG_OPTIONS
+#  define GRP 10
+     {NULL, 0, NULL, 0, "System options are:", GRP},
+     SYSTEM_LONG_OPTIONS
+#  undef GRP
+#endif
+  {NULL}
+};
+
 /* Be verbose about actions.  */
 int verbose;
 
 /* Array of all interfaces on the command line.  */
 struct ifconfig *ifs;
 
+static struct ifconfig *ifp;
+
 /* Size of IFS.  */
 int nifs;
 
@@ -141,7 +181,7 @@ static const char *short_options = "+i:a
 
 static struct option long_options[] = {
 #ifdef SYSTEM_LONG_OPTIONS
-  SYSTEM_LONG_OPTIONS
+  //  SYSTEM_LONG_OPTIONS
 #endif
   {"verbose", no_argument, 0, 'v'},
   {"version", no_argument, 0, 'V'},
@@ -164,7 +204,8 @@ usage (int err)
 {
   if (err != EXIT_SUCCESS)
     {
-      fprintf (stderr, "Usage: %s [OPTION]...%s\n", program_name,
+      fprintf (stderr, "Usage: %s [OPTION]...%s\n",
+              program_name,
               system_help ? " [SYSTEM OPTION]..." : "");
       fprintf (stderr, "Try `%s --help' for more information.\n",
               program_name);
@@ -232,7 +273,7 @@ parse_opt_set_##field (struct ifconfig *
     {                                                          \
       fprintf (stderr, "%s: no interface specified for " #fname        \
               " `%s'\n", program_name, addr);                  \
-      usage (EXIT_FAILURE);                                    \
+      usage (EXIT_FAILURE);                            \
     }                                                          \
   if (ifp->valid & IF_VALID_##fvalid)                          \
     {                                                          \
@@ -258,7 +299,7 @@ parse_opt_set_##field (struct ifconfig *
     {                                                          \
       fprintf (stderr, "%s: no interface specified for " #fname        \
               " `%s'\n", program_name, arg);                   \
-      usage (EXIT_FAILURE);                                    \
+      usage (EXIT_FAILURE);                            \
     }                                                          \
   if (ifp->valid & IF_VALID_##fvalid)                          \
     {                                                          \
@@ -335,87 +376,16 @@ parse_opt_finalize (struct ifconfig *ifp
     }
 }
 
-char *program_name;
-
 void
-parse_opt (int argc, char *argv[])
+parse_opt_success (struct argp_state *state)
 {
-  int option;
-  struct ifconfig *ifp = ifs;
-
-  program_name = argv[0];
-
-  parse_opt_set_default_format (NULL);
-
-  while ((option = getopt_long (argc, argv, short_options,
-                               long_options, 0)) != EOF)
-    {
-      /* XXX: Allow new ifs be created by system_parse_opt. Provide
-         helper function for that (esp necessary for system specific
-         parsing of remaining args.  */
-      if (system_parse_opt (&ifp, option, optarg))
-       continue;
-
-      switch (option)
-       {
-       case 'i':               /* Interface name.  */
-         parse_opt_finalize (ifp);
-         ifp = parse_opt_new_ifs (optarg);
-         break;
-
-       case 'a':               /* Interface address.  */
-         parse_opt_set_address (ifp, optarg);
-         break;
-
-       case 'm':               /* Interface netmask.  */
-         parse_opt_set_netmask (ifp, optarg);
-         break;
-
-       case 'd':               /* Interface dstaddr.  */
-       case 'p':
-         parse_opt_set_dstaddr (ifp, optarg);
-         break;
-
-       case 'b':               /* Interface broadcast address.  */
-       case 'B':
-         parse_opt_set_brdaddr (ifp, optarg);
-         break;
-
-       case 'M':               /* Interface MTU.  */
-         parse_opt_set_mtu (ifp, optarg);
-         break;
-
-       case '3':               /* Interface metric.  */
-         parse_opt_set_metric (ifp, optarg);
-         break;
-
-       case '4':               /* Output format.  */
-         parse_opt_set_default_format (optarg);
-         break;
-
-       case 'v':               /* Verbose.  */
-         verbose = 1;
-         break;
-
-       case '&':               /* Help.  */
-         usage (EXIT_SUCCESS);
-         /* Not reached.  */
-
-       case 'V':               /* Version.  */
-         printf ("ifconfig (%s) %s\n", PACKAGE_NAME, PACKAGE_VERSION);
-         exit (EXIT_SUCCESS);
-
-       case '?':
-       default:
-         usage (EXIT_FAILURE);
-         /* Not reached.  */
-       }
-    }
   parse_opt_finalize (ifp);
 
-  if (optind < argc)
+  if (state->next < state->argc)
     {
-      if (!system_parse_opt_rest (&ifp, argc - optind, &argv[optind]))
+      if (!system_parse_opt_rest (&ifp,
+                                 state->argc - state->next + 1,
+                                 &state->argv[state->next - 1]))
        usage (EXIT_FAILURE);
       parse_opt_finalize (ifp);
     }
@@ -448,3 +418,69 @@ parse_opt (int argc, char *argv[])
          (if->if_name) here.  */
     }
 }
+
+error_t
+parse_opt (int key, char *arg, struct argp_state *state)
+{
+  if (system_parse_opt (&ifp, key, arg))
+    return 0;
+  
+  switch (key)
+    {
+    case ARGP_KEY_INIT:
+      ifp = ifs;
+      parse_opt_set_default_format (NULL);
+      break;
+
+    case 'a':
+      parse_opt_set_address (ifp, arg);
+      break;
+
+    case 'b':
+    case 'B':
+      parse_opt_set_brdaddr (ifp, arg);
+      break;
+
+    case 'd':
+    case 'p':
+      parse_opt_set_dstaddr (ifp, arg);
+      break;
+
+    case 'i':
+      parse_opt_finalize (ifp);
+      ifp = parse_opt_new_ifs (arg);
+      break;
+
+    case 'm':
+      parse_opt_set_netmask (ifp, arg);
+      break;
+
+    case 'M':
+      parse_opt_set_mtu (ifp, arg);
+      break;
+
+    case 'v':
+      verbose = 1;
+      break;
+
+    case ARG_METRIC:
+      parse_opt_set_metric (ifp, arg);
+      break;
+
+    case ARG_FORMAT:
+      parse_opt_set_default_format (arg);
+      break;
+
+    case ARGP_KEY_ARG:
+      parse_opt_success (state);
+      break;
+
+    default:
+      return ARGP_ERR_UNKNOWN;
+    }
+
+  return 0;
+}
+
+struct argp argp = {argp_options, parse_opt, args_doc, doc};
Index: options.h
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ifconfig/options.h,v
retrieving revision 1.5
diff -p -u -r1.5 options.h
--- options.h   21 Oct 2006 15:24:20 -0000      1.5
+++ options.h   4 Apr 2007 18:37:46 -0000
@@ -22,6 +22,8 @@
 #ifndef IFCONFIG_OPTIONS_H
 # define IFCONFIG_OPTIONS_H
 
+#include <argp.h>
+
 # include <sys/types.h>
 # include <sys/socket.h>
 
@@ -70,7 +72,8 @@ extern int nifs;
 /* Be verbose about what we do.  */
 extern int verbose;
 
-void usage (int err);
+extern struct argp argp;
+
 void parse_opt_set_address (struct ifconfig *ifp, char *addr);
 void parse_opt_set_brdaddr (struct ifconfig *ifp, char *addr);
 void parse_opt_set_dstaddr (struct ifconfig *ifp, char *addr);
@@ -80,6 +83,7 @@ void parse_opt_set_metric (struct ifconf
 void parse_opt_set_default_format (const char *format);
 void parse_opt_finalize (struct ifconfig *ifp);
 
-void parse_opt (int argc, char *argv[]);
+void parse_opt_success (struct argp_state *state);
+error_t parse_opt (int key, char *arg, struct argp_state *state);
 
 #endif
Index: system/linux.h
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ifconfig/system/linux.h,v
retrieving revision 1.3
diff -p -u -r1.3 linux.h
--- system/linux.h      21 Oct 2006 15:24:20 -0000      1.3
+++ system/linux.h      4 Apr 2007 18:37:46 -0000
@@ -37,8 +37,7 @@ struct system_ifconfig
 };
 
 # define SYSTEM_LONG_OPTIONS \
-  {"txqlen",         required_argument,      0,      'T'},
-
+  {"txqlen", 'T', "N", 0, "Set transmit queue length to N", GRP+1},
 
 /* Output format support.  */