Prefer named constants over magic numbers in the 'struct option' list, and expand the list of enums for long-only options so that the call to getopt_long() can switch directly to every option, rather than needing a lengthy if/else chain that grows for every new long option. Patch best viewed with whitespace changes ignored. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/main.c | 191 ++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 100 insertions(+), 91 deletions(-) diff --git a/src/main.c b/src/main.c index d2e5674..6be0991 100644 --- a/src/main.c +++ b/src/main.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2017 Red Hat Inc. + * Copyright (C) 2013-2018 Red Hat Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -109,44 +109,55 @@ struct backend *backend; static char *random_fifo_dir = NULL; static char *random_fifo = NULL; -enum { HELP_OPTION = CHAR_MAX + 1 }; +enum { + HELP_OPTION = CHAR_MAX + 1, + DUMP_CONFIG_OPTION, + DUMP_PLUGIN_OPTION, + EXIT_WITH_PARENT_OPTION, + FILTER_OPTION, + RUN_OPTION, + SELINUX_LABEL_OPTION, + TLS_OPTION, + TLS_CERTIFICATES_OPTION, + TLS_VERIFY_PEER_OPTION, +}; static const char *short_options = "e:fg:i:nop:P:rst:u:U:vV"; static const struct option long_options[] = { - { "help", 0, NULL, HELP_OPTION }, - { "dump-config",0, NULL, 0 }, - { "dump-plugin",0, NULL, 0 }, - { "exit-with-parent", 0, NULL, 0 }, - { "export", 1, NULL, 'e' }, - { "export-name",1, NULL, 'e' }, - { "exportname", 1, NULL, 'e' }, - { "filter", 1, NULL, 0 }, - { "foreground", 0, NULL, 'f' }, - { "no-fork", 0, NULL, 'f' }, - { "group", 1, NULL, 'g' }, - { "ip-addr", 1, NULL, 'i' }, - { "ipaddr", 1, NULL, 'i' }, - { "new-style", 0, NULL, 'n' }, - { "newstyle", 0, NULL, 'n' }, - { "old-style", 0, NULL, 'o' }, - { "oldstyle", 0, NULL, 'o' }, - { "pid-file", 1, NULL, 'P' }, - { "pidfile", 1, NULL, 'P' }, - { "port", 1, NULL, 'p' }, - { "read-only", 0, NULL, 'r' }, - { "readonly", 0, NULL, 'r' }, - { "run", 1, NULL, 0 }, - { "selinux-label", 1, NULL, 0 }, - { "single", 0, NULL, 's' }, - { "stdin", 0, NULL, 's' }, - { "threads", 1, NULL, 't' }, - { "tls", 1, NULL, 0 }, - { "tls-certificates", 1, NULL, 0 }, - { "tls-verify-peer", 0, NULL, 0 }, - { "unix", 1, NULL, 'U' }, - { "user", 1, NULL, 'u' }, - { "verbose", 0, NULL, 'v' }, - { "version", 0, NULL, 'V' }, + { "help", no_argument, NULL, HELP_OPTION }, + { "dump-config", no_argument, NULL, DUMP_CONFIG_OPTION }, + { "dump-plugin", no_argument, NULL, DUMP_PLUGIN_OPTION }, + { "exit-with-parent", no_argument, NULL, EXIT_WITH_PARENT_OPTION }, + { "export", required_argument, NULL, 'e' }, + { "export-name", required_argument, NULL, 'e' }, + { "exportname", required_argument, NULL, 'e' }, + { "filter", required_argument, NULL, FILTER_OPTION }, + { "foreground", no_argument, NULL, 'f' }, + { "no-fork", no_argument, NULL, 'f' }, + { "group", required_argument, NULL, 'g' }, + { "ip-addr", required_argument, NULL, 'i' }, + { "ipaddr", required_argument, NULL, 'i' }, + { "new-style", no_argument, NULL, 'n' }, + { "newstyle", no_argument, NULL, 'n' }, + { "old-style", no_argument, NULL, 'o' }, + { "oldstyle", no_argument, NULL, 'o' }, + { "pid-file", required_argument, NULL, 'P' }, + { "pidfile", required_argument, NULL, 'P' }, + { "port", required_argument, NULL, 'p' }, + { "read-only", no_argument, NULL, 'r' }, + { "readonly", no_argument, NULL, 'r' }, + { "run", required_argument, NULL, RUN_OPTION }, + { "selinux-label", required_argument, NULL, SELINUX_LABEL_OPTION }, + { "single", no_argument, NULL, 's' }, + { "stdin", no_argument, NULL, 's' }, + { "threads", required_argument, NULL, 't' }, + { "tls", required_argument, NULL, TLS_OPTION }, + { "tls-certificates", required_argument, NULL, TLS_CERTIFICATES_OPTION }, + { "tls-verify-peer", no_argument, NULL, TLS_VERIFY_PEER_OPTION }, + { "unix", required_argument, NULL, 'U' }, + { "user", required_argument, NULL, 'u' }, + { "verbose", no_argument, NULL, 'v' }, + { "version", no_argument, NULL, 'V' }, { NULL }, }; @@ -233,25 +244,27 @@ main (int argc, char *argv[]) break; switch (c) { - case 0: /* options which are long only */ - if (strcmp (long_options[option_index].name, "dump-config") == 0) { - dump_config (); - exit (EXIT_SUCCESS); - } - else if (strcmp (long_options[option_index].name, "dump-plugin") == 0) { - dump_plugin = 1; - } - else if (strcmp (long_options[option_index].name, "exit-with-parent") == 0) { + case DUMP_CONFIG_OPTION: + dump_config (); + exit (EXIT_SUCCESS); + + case DUMP_PLUGIN_OPTION: + dump_plugin = 1; + break; + + case EXIT_WITH_PARENT_OPTION: #ifdef PR_SET_PDEATHSIG - exit_with_parent = 1; - foreground = 1; + exit_with_parent = 1; + foreground = 1; + break; #else - fprintf (stderr, "%s: --exit-with-parent is not implemented for this operating system\n", - program_name); - exit (EXIT_FAILURE); + fprintf (stderr, "%s: --exit-with-parent is not implemented for this operating system\n", + program_name); + exit (EXIT_FAILURE); #endif - } - else if (strcmp (long_options[option_index].name, "filter") == 0) { + + case FILTER_OPTION: + { struct filter_filename *t; t = malloc (sizeof *t); @@ -263,51 +276,47 @@ main (int argc, char *argv[]) t->filename = optarg; filter_filenames = t; } - else if (strcmp (long_options[option_index].name, "run") == 0) { - if (socket_activation) { - fprintf (stderr, "%s: cannot use socket activation with --run flag\n", - program_name); - exit (EXIT_FAILURE); - } - run = optarg; - foreground = 1; - } - else if (strcmp (long_options[option_index].name, "selinux-label") == 0) { - selinux_label = optarg; - break; - } - else if (strcmp (long_options[option_index].name, "tls") == 0) { - tls_set_on_cli = 1; - if (strcmp (optarg, "off") == 0 || strcmp (optarg, "0") == 0) - tls = 0; - else if (strcmp (optarg, "on") == 0 || strcmp (optarg, "1") == 0) - tls = 1; - else if (strcmp (optarg, "require") == 0 || - strcmp (optarg, "required") == 0 || - strcmp (optarg, "force") == 0) - tls = 2; - else { - fprintf (stderr, "%s: --tls flag must be off|on|require\n", - program_name); - exit (EXIT_FAILURE); - } - break; - } - else if (strcmp (long_options[option_index].name, "tls-certificates") == 0) { - tls_certificates_dir = optarg; - break; - } - else if (strcmp (long_options[option_index].name, "tls-verify-peer") == 0) { - tls_verify_peer = 1; - break; + break; + + case RUN_OPTION: + if (socket_activation) { + fprintf (stderr, "%s: cannot use socket activation with --run flag\n", + program_name); + exit (EXIT_FAILURE); } + run = optarg; + foreground = 1; + break; + + case SELINUX_LABEL_OPTION: + selinux_label = optarg; + break; + + case TLS_OPTION: + tls_set_on_cli = 1; + if (strcmp (optarg, "off") == 0 || strcmp (optarg, "0") == 0) + tls = 0; + else if (strcmp (optarg, "on") == 0 || strcmp (optarg, "1") == 0) + tls = 1; + else if (strcmp (optarg, "require") == 0 || + strcmp (optarg, "required") == 0 || + strcmp (optarg, "force") == 0) + tls = 2; else { - fprintf (stderr, "%s: unknown long option: %s (%d)\n", - program_name, long_options[option_index].name, option_index); + fprintf (stderr, "%s: --tls flag must be off|on|require\n", + program_name); exit (EXIT_FAILURE); } break; + case TLS_CERTIFICATES_OPTION: + tls_certificates_dir = optarg; + break; + + case TLS_VERIFY_PEER_OPTION: + tls_verify_peer = 1; + break; + case 'e': exportname = optarg; newstyle = 1; -- 2.14.4