Re: [bug-inetutils] Argpifying ping.

[Alfred M. Szmidt]

Comments here they come...

   diff -urNp ../inetutils/ping/ping.c ping/ping.c
   --- ../inetutils/ping/ping.c 2007-03-29 23:28:43.000000000 +0530
   +++ ping/ping.c      2007-03-30 01:36:42.000000000 +0530
   @@ -46,234 +46,208 @@
    #include <errno.h>
    #include <limits.h>

   +#include <argp.h>
    #include <getopt.h>
    #include <icmp.h>
    #include <ping.h>
    #include "ping_common.h"
    #include "ping_impl.h"

   -static char short_options[] = "VLhc:dfi:l:np:qRrs:t:v";
   -static struct option long_options[] = {
   -  /* Help options */
   -  {"version", no_argument, NULL, 'V'},
   -  {"license", no_argument, NULL, 'L'},
   -  {"help", no_argument, NULL, 'h'},
   -  /* Common options */
   -  {"count", required_argument, NULL, 'c'},
   -  {"debug", no_argument, NULL, 'd'},
   -  {"ignore-routing", no_argument, NULL, 'r'},
   -  {"size", required_argument, NULL, 's'},
   -  {"interval", required_argument, NULL, 'i'},
   -  {"numeric", no_argument, NULL, 'n'},
   -  {"verbose", no_argument, NULL, 'v'},
   -  /* Packet types */
   -  {"type", required_argument, NULL, 't'},
   -  {"echo", no_argument, NULL, ICMP_ECHO},
   -  {"timestamp", no_argument, NULL, ICMP_TIMESTAMP},
   -  {"address", no_argument, NULL, ICMP_ADDRESS},
   -  {"router", no_argument, NULL, ICMP_ROUTERDISCOVERY},
   -  /* echo-specific options */
   -  {"flood", no_argument, NULL, 'f'},
   -  {"preload", required_argument, NULL, 'l'},
   -  {"pattern", required_argument, NULL, 'p'},
   -  {"quiet", no_argument, NULL, 'q'},
   -  {"route", no_argument, NULL, 'R'},
   -  {NULL, no_argument, NULL, 0}
   -};
   -
    extern int ping_echo (int argc, char **argv);
    extern int ping_timestamp (int argc, char **argv);
    extern int ping_address (int argc, char **argv);
    extern int ping_router (int argc, char **argv);

    PING *ping;
   +bool is_root = false;
    u_char *data_buffer;
   +u_char *patptr;
   +int pattern_len = 16;
   +int socket_type;
   +size_t count = DEFAULT_PING_COUNT;
   +size_t interval;
    size_t data_length = PING_DATALEN;
    unsigned options;
    unsigned long preload = 0;
    int (*ping_type) (int argc, char **argv) = ping_echo;

   -
   -static void show_usage (void);
   -static void decode_type (const char *optarg);
   +/*static void decode_type (const char *optarg);*/
   +int (*decode_type (const char *optarg)) (int argc, char **argv);
    static int send_echo (PING * ping);

    #define MIN_USER_INTERVAL (200000/PING_PRECISION)

   -char *program_name;
   +const char *argp_program_bug_address = "<" PACKAGE_BUGREPORT ">";

Maybe this should be a macro, or define in one place?

   +
   +const char *argp_program_version = "ping (" PACKAGE_NAME ") " 
PACKAGE_VERSION
   +"\nThis is free software.  You may redistribute copies of it under
   the terms of"
   +"\nthe GNU General Public License <http://www.gnu.org/licenses/gpl.html>."
   +"\nThere is NO WARRANTY, to the extent permitted by law."

Same with this, this is a static blurb that might or might not change
once in a blue moon.

   +"\n"
   +"\nWritten by Sergey Poznyakoff.";

Otherwise it looks OK, have you tested it and such?

   diff -urNp ../inetutils/ping/ping_impl.h ping/ping_impl.h
   --- ../inetutils/ping/ping_impl.h    2006-10-21 20:54:20.000000000 +0530
   +++ ping/ping_impl.h 2007-03-30 00:25:59.000000000 +0530
   @@ -24,7 +24,7 @@ struct ping_stat

    extern unsigned options;
    extern PING *ping;
   -extern int is_root;
   +/*extern int is_root;*/
    extern unsigned long preload;
    extern u_char *data_buffer;
    extern size_t data_length;

Seems to be safe to remove that extern... It is only used inside
ping6.c and ping.c (and they can't be shared).