From e62fd508158749a0d55eae06c2e361df5d6da6e0 Mon Sep 17 00:00:00 2001 From: Kirill Isakov Date: Tue, 26 Apr 2022 19:51:23 +0600 Subject: [PATCH] Use enums for command-line options to avoid repeating options in multiple places, get more descriptive names, and have the compiler verify the exhaustiveness of reading options for us. --- src/sptps_keypair.c | 19 ++++++--- src/sptps_test.c | 78 +++++++++++++++++++++++------------- src/tincctl.c | 60 ++++++++++++++++++---------- src/tincd.c | 97 ++++++++++++++++++++++++++++----------------- 4 files changed, 161 insertions(+), 93 deletions(-) diff --git a/src/sptps_keypair.c b/src/sptps_keypair.c index 7fcfee64..97d51895 100644 --- a/src/sptps_keypair.c +++ b/src/sptps_keypair.c @@ -45,9 +45,16 @@ static void usage(void) { fprintf(stderr, "Report bugs to tinc@tinc-vpn.org.\n"); } +typedef enum option_t { + OPT_BAD_OPTION = '?', + OPT_LONG_OPTION = 0, + + OPT_HELP = 255, +} option_t; + static struct option const long_options[] = { - {"help", no_argument, NULL, 1}, - {NULL, 0, NULL, 0} + {"help", no_argument, NULL, OPT_HELP}, + {NULL, 0, NULL, 0} }; static int generate_keypair(char *argv[]) { @@ -96,15 +103,15 @@ int main(int argc, char *argv[]) { int option_index = 0; while((r = getopt_long(argc, argv, "", long_options, &option_index)) != EOF) { - switch(r) { - case 0: /* long option */ + switch((option_t) r) { + case OPT_LONG_OPTION: break; - case '?': /* wrong options */ + case OPT_BAD_OPTION: usage(); return 1; - case 1: /* help */ + case OPT_HELP: usage(); return 0; diff --git a/src/sptps_test.c b/src/sptps_test.c index acc692ad..b26615e1 100644 --- a/src/sptps_test.c +++ b/src/sptps_test.c @@ -118,17 +118,39 @@ static bool receive_record(void *handle, uint8_t type, const void *data, uint16_ return true; } +typedef enum option_t { + OPT_BAD_OPTION = '?', + OPT_LONG_OPTION = 0, + + // Short options + OPT_DATAGRAM = 'd', + OPT_QUIT_ON_EOF = 'q', + OPT_READONLY = 'r', + OPT_WRITEONLY = 'w', + OPT_PACKET_LOSS = 'L', + OPT_REPLAY_WINDOW = 'W', + OPT_SPECIAL_CHAR = 's', + OPT_TUN = 't', + OPT_VERBOSE = 'v', + OPT_IPV4 = '4', + OPT_IPV6 = '6', + + // Long options + OPT_HELP = 255, +} option_t; + static struct option const long_options[] = { - {"datagram", no_argument, NULL, 'd'}, - {"quit", no_argument, NULL, 'q'}, - {"readonly", no_argument, NULL, 'r'}, - {"writeonly", no_argument, NULL, 'w'}, - {"packet-loss", required_argument, NULL, 'L'}, - {"replay-window", required_argument, NULL, 'W'}, - {"special", no_argument, NULL, 's'}, - {"verbose", required_argument, NULL, 'v'}, - {"help", no_argument, NULL, 1}, - {NULL, 0, NULL, 0} + {"datagram", no_argument, NULL, OPT_DATAGRAM}, + {"quit", no_argument, NULL, OPT_QUIT_ON_EOF}, + {"readonly", no_argument, NULL, OPT_READONLY}, + {"writeonly", no_argument, NULL, OPT_WRITEONLY}, + {"packet-loss", required_argument, NULL, OPT_PACKET_LOSS}, + {"replay-window", required_argument, NULL, OPT_REPLAY_WINDOW}, + {"special", no_argument, NULL, OPT_SPECIAL_CHAR}, + {"tun", no_argument, NULL, OPT_TUN}, + {"verbose", required_argument, NULL, OPT_VERBOSE}, + {"help", no_argument, NULL, OPT_HELP}, + {NULL, 0, NULL, 0} }; static void usage(void) { @@ -328,23 +350,27 @@ static int run_test(int argc, char *argv[]) { bool quit = false; while((r = getopt_long(argc, argv, "dqrstwL:W:v46", long_options, &option_index)) != EOF) { - switch(r) { - case 0: /* long option */ + switch((option_t) r) { + case OPT_LONG_OPTION: break; - case 'd': /* datagram mode */ + case OPT_BAD_OPTION: + usage(); + return 1; + + case OPT_DATAGRAM: datagram = true; break; - case 'q': /* close connection on EOF from stdin */ + case OPT_QUIT_ON_EOF: quit = true; break; - case 'r': /* read only */ + case OPT_READONLY: readonly = true; break; - case 't': /* read only */ + case OPT_TUN: #ifdef HAVE_LINUX tun = true; #else @@ -354,39 +380,35 @@ static int run_test(int argc, char *argv[]) { #endif break; - case 'w': /* write only */ + case OPT_WRITEONLY: writeonly = true; break; - case 'L': /* packet loss rate */ + case OPT_PACKET_LOSS: packetloss = atoi(optarg); break; - case 'W': /* replay window size */ + case OPT_REPLAY_WINDOW: sptps_replaywin = atoi(optarg); break; - case 'v': /* be verbose */ + case OPT_VERBOSE: verbose = true; break; - case 's': /* special character handling */ + case OPT_SPECIAL_CHAR: special = true; break; - case '?': /* wrong options */ - usage(); - return 1; - - case '4': /* IPv4 */ + case OPT_IPV4: addressfamily = AF_INET; break; - case '6': /* IPv6 */ + case OPT_IPV6: addressfamily = AF_INET6; break; - case 1: /* help */ + case OPT_HELP: usage(); return 0; diff --git a/src/tincctl.c b/src/tincctl.c index a882a668..a18f6ce6 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -80,15 +80,31 @@ char *device = NULL; char *iface = NULL; int debug_level = -1; +typedef enum option_t { + OPT_BAD_OPTION = '?', + OPT_LONG_OPTION = 0, + + // Short options + OPT_BATCH = 'b', + OPT_CONFIG_FILE = 'c', + OPT_NETNAME = 'n', + + // Long options + OPT_HELP = 255, + OPT_VERSION, + OPT_PIDFILE, + OPT_FORCE, +} option_t; + static struct option const long_options[] = { - {"batch", no_argument, NULL, 'b'}, - {"config", required_argument, NULL, 'c'}, - {"net", required_argument, NULL, 'n'}, - {"help", no_argument, NULL, 1}, - {"version", no_argument, NULL, 2}, - {"pidfile", required_argument, NULL, 3}, - {"force", no_argument, NULL, 4}, - {NULL, 0, NULL, 0} + {"batch", no_argument, NULL, OPT_BATCH}, + {"config", required_argument, NULL, OPT_CONFIG_FILE}, + {"net", required_argument, NULL, OPT_NETNAME}, + {"help", no_argument, NULL, OPT_HELP}, + {"version", no_argument, NULL, OPT_VERSION}, + {"pidfile", required_argument, NULL, OPT_PIDFILE}, + {"force", no_argument, NULL, OPT_FORCE}, + {NULL, 0, NULL, 0}, }; static void version(void) { @@ -189,47 +205,47 @@ static bool parse_options(int argc, char **argv) { int option_index = 0; while((r = getopt_long(argc, argv, "+bc:n:", long_options, &option_index)) != EOF) { - switch(r) { - case 0: /* long option */ + switch((option_t) r) { + case OPT_LONG_OPTION: break; - case 'b': + case OPT_BAD_OPTION: + usage(true); + free_names(); + return false; + + case OPT_BATCH: tty = false; break; - case 'c': /* config file */ + case OPT_CONFIG_FILE: free(confbase); confbase = xstrdup(optarg); confbasegiven = true; break; - case 'n': /* net name given */ + case OPT_NETNAME: free(netname); netname = xstrdup(optarg); break; - case 1: /* show help */ + case OPT_HELP: show_help = true; break; - case 2: /* show version */ + case OPT_VERSION: show_version = true; break; - case 3: /* open control socket here */ + case OPT_PIDFILE: free(pidfilename); pidfilename = xstrdup(optarg); break; - case 4: /* force */ + case OPT_FORCE: force = true; break; - case '?': /* wrong options */ - usage(true); - free_names(); - return false; - default: break; } diff --git a/src/tincd.c b/src/tincd.c index 231e01fa..5bfeeabb 100644 --- a/src/tincd.c +++ b/src/tincd.c @@ -79,22 +79,45 @@ char **g_argv; /* a copy of the cmdline arguments */ static int status = 1; +typedef enum option_t { + OPT_BAD_OPTION = '?', + OPT_LONG_OPTION = 0, + + // Short options + OPT_CONFIG_FILE = 'c', + OPT_NETNAME = 'n', + OPT_NO_DETACH = 'D', + OPT_DEBUG = 'd', + OPT_MLOCK = 'L', + OPT_CHROOT = 'R', + OPT_CHANGE_USER = 'U', + OPT_SYSLOG = 's', + OPT_OPTION = 'o', + + // Long options + OPT_HELP = 255, + OPT_VERSION, + OPT_NO_SECURITY, + OPT_LOGFILE, + OPT_PIDFILE, +} option_t; + static struct option const long_options[] = { - {"config", required_argument, NULL, 'c'}, - {"net", required_argument, NULL, 'n'}, - {"help", no_argument, NULL, 1}, - {"version", no_argument, NULL, 2}, - {"no-detach", no_argument, NULL, 'D'}, - {"debug", optional_argument, NULL, 'd'}, - {"bypass-security", no_argument, NULL, 3}, - {"mlock", no_argument, NULL, 'L'}, - {"chroot", no_argument, NULL, 'R'}, - {"user", required_argument, NULL, 'U'}, - {"logfile", optional_argument, NULL, 4}, - {"syslog", no_argument, NULL, 's'}, - {"pidfile", required_argument, NULL, 5}, - {"option", required_argument, NULL, 'o'}, - {NULL, 0, NULL, 0} + {"config", required_argument, NULL, OPT_CONFIG_FILE}, + {"net", required_argument, NULL, OPT_NETNAME}, + {"no-detach", no_argument, NULL, OPT_NO_DETACH}, + {"debug", optional_argument, NULL, OPT_DEBUG}, + {"mlock", no_argument, NULL, OPT_MLOCK}, + {"chroot", no_argument, NULL, OPT_CHROOT}, + {"user", required_argument, NULL, OPT_CHANGE_USER}, + {"syslog", no_argument, NULL, OPT_SYSLOG}, + {"option", required_argument, NULL, OPT_OPTION}, + {"help", no_argument, NULL, OPT_HELP}, + {"version", no_argument, NULL, OPT_VERSION}, + {"bypass-security", no_argument, NULL, OPT_NO_SECURITY}, + {"logfile", optional_argument, NULL, OPT_LOGFILE}, + {"pidfile", required_argument, NULL, OPT_PIDFILE}, + {NULL, 0, NULL, 0}, }; #ifdef HAVE_WINDOWS @@ -142,20 +165,24 @@ static bool parse_options(int argc, char **argv) { int lineno = 0; while((r = getopt_long(argc, argv, "c:DLd::n:so:RU:", long_options, &option_index)) != EOF) { - switch(r) { - case 0: /* long option */ + switch((option_t) r) { + case OPT_LONG_OPTION: break; - case 'c': /* config file */ + case OPT_BAD_OPTION: + usage(true); + goto exit_fail; + + case OPT_CONFIG_FILE: free(confbase); confbase = xstrdup(optarg); break; - case 'D': /* no detach */ + case OPT_NO_DETACH: do_detach = false; break; - case 'L': /* no detach */ + case OPT_MLOCK: /* lock tincd into RAM */ #ifndef HAVE_MLOCKALL logger(DEBUG_ALWAYS, LOG_ERR, "The %s option is not supported on this platform.", argv[optind - 1]); goto exit_fail; @@ -164,7 +191,7 @@ static bool parse_options(int argc, char **argv) { break; #endif - case 'd': /* increase debug level */ + case OPT_DEBUG: /* increase debug level */ if(!optarg && optind < argc && *argv[optind] != '-') { optarg = argv[optind++]; } @@ -177,17 +204,17 @@ static bool parse_options(int argc, char **argv) { break; - case 'n': /* net name given */ + case OPT_NETNAME: free(netname); netname = xstrdup(optarg); break; - case 's': /* syslog */ + case OPT_SYSLOG: use_logfile = false; use_syslog = true; break; - case 'o': /* option */ + case OPT_OPTION: cfg = parse_config_line(optarg, NULL, ++lineno); if(!cfg) { @@ -199,34 +226,34 @@ static bool parse_options(int argc, char **argv) { #ifdef HAVE_WINDOWS - case 'R': - case 'U': + case OPT_CHANGE_USER: + case OPT_CHROOT: logger(DEBUG_ALWAYS, LOG_ERR, "The %s option is not supported on this platform.", argv[optind - 1]); goto exit_fail; #else - case 'R': /* chroot to NETNAME dir */ + case OPT_CHROOT: do_chroot = true; break; - case 'U': /* setuid to USER */ + case OPT_CHANGE_USER: switchuser = optarg; break; #endif - case 1: /* show help */ + case OPT_HELP: show_help = true; break; - case 2: /* show version */ + case OPT_VERSION: show_version = true; break; - case 3: /* bypass security */ + case OPT_NO_SECURITY: bypass_security = true; break; - case 4: /* write log entries to a file */ + case OPT_LOGFILE: use_syslog = false; use_logfile = true; @@ -241,15 +268,11 @@ static bool parse_options(int argc, char **argv) { break; - case 5: /* open control socket here */ + case OPT_PIDFILE: free(pidfilename); pidfilename = xstrdup(optarg); break; - case '?': /* wrong options */ - usage(true); - goto exit_fail; - default: break; } -- 2.20.1