From: Kirill Isakov <bootctl@gmail.com>
Date: Tue, 26 Apr 2022 13:51:23 +0000 (+0600)
Subject: Use enums for command-line options
X-Git-Url: https://www.tinc-vpn.org/git/browse?p=tinc;a=commitdiff_plain;h=e62fd508158749a0d55eae06c2e361df5d6da6e0

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.
---

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;
 		}