On Fri, Apr 14, 2023 at 09:59:53AM +0200, Laszlo Ersek wrote: > Two of the enum constants that denote command line options are > inconsistently named with the rest: all identifiers should be > <purpose>_OPTION, but LONG_OPTIONS and SHORT_OPTIONS (which are supposed > to list the long and short options) don't conform. Rename them. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 > Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; > --- > copy/main.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/copy/main.c b/copy/main.c > index f9a714c4f677..bb67e97ff97a 100644 > --- a/copy/main.c > +++ b/copy/main.c > @@ -107,8 +107,8 @@ main (int argc, char *argv[]) > { > enum { > HELP_OPTION = CHAR_MAX + 1, > - LONG_OPTIONS, > - SHORT_OPTIONS, > + LONG_OPTIONS_OPTION, > + SHORT_OPTIONS_OPTION, > ALLOCATED_OPTION, > DESTINATION_IS_ZERO_OPTION, > FLUSH_OPTION, > @@ -120,7 +120,7 @@ main (int argc, char *argv[]) > const char *short_options = "C:pR:S:T:vV"; > const struct option long_options[] = { > { "help", no_argument, NULL, HELP_OPTION }, > - { "long-options", no_argument, NULL, LONG_OPTIONS }, > + { "long-options", no_argument, NULL, LONG_OPTIONS_OPTION }, > { "allocated", no_argument, NULL, ALLOCATED_OPTION }, > { "connections", required_argument, NULL, 'C' }, > { "destination-is-zero",no_argument, NULL, DESTINATION_IS_ZERO_OPTION }, > @@ -130,7 +130,7 @@ main (int argc, char *argv[]) > { "queue-size", required_argument, NULL, QUEUE_SIZE_OPTION }, > { "request-size", required_argument, NULL, REQUEST_SIZE_OPTION }, > { "requests", required_argument, NULL, 'R' }, > - { "short-options", no_argument, NULL, SHORT_OPTIONS }, > + { "short-options", no_argument, NULL, SHORT_OPTIONS_OPTION }, > { "sparse", required_argument, NULL, 'S' }, > { "synchronous", no_argument, NULL, SYNCHRONOUS_OPTION }, > { "target-is-zero", no_argument, NULL, DESTINATION_IS_ZERO_OPTION }, > @@ -155,7 +155,7 @@ main (int argc, char *argv[]) > case HELP_OPTION: > usage (stdout, EXIT_SUCCESS); > > - case LONG_OPTIONS: > + case LONG_OPTIONS_OPTION: > for (i = 0; long_options[i].name != NULL; ++i) { > if (strcmp (long_options[i].name, "long-options") != 0 && > strcmp (long_options[i].name, "short-options") != 0) > @@ -163,7 +163,7 @@ main (int argc, char *argv[]) > } > exit (EXIT_SUCCESS); > > - case SHORT_OPTIONS: > + case SHORT_OPTIONS_OPTION: > for (i = 0; short_options[i]; ++i) { > if (short_options[i] != ':' && short_options[i] != '+') > printf ("-%c\n", short_options[i]); Assuming (from the cover message) that the same will be done later for nbdinfo, fuse etc then ACK.

From these, "dump/dump.c" and "info/main.c" don't exceed 80 chars, so I

I was going to say that we use LONG_OPTIONS & SHORT_OPTIONS elsewhere, but actually we don't. We use --long-options and --short-options extensively (eg. in guestfs-tools) but those are implemented differently. Rich.

Rename <purpose>_OPTION to OPT_<purpose> for two reasons: - it is more idiomatic for enum constants to have the same prefix than for them to have the same suffix, - we hew away three characters (length-wise) from each option name, which happens to reduce the max width of "copy/main.c" to 79 characters. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- copy/main.c | 62 ++++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/copy/main.c b/copy/main.c index bb67e97ff97a..8055b3a656ba 100644 --- a/copy/main.c +++ b/copy/main.c @@ -106,34 +106,34 @@ int main (int argc, char *argv[]) { enum { - HELP_OPTION = CHAR_MAX + 1, - LONG_OPTIONS_OPTION, - SHORT_OPTIONS_OPTION, - ALLOCATED_OPTION, - DESTINATION_IS_ZERO_OPTION, - FLUSH_OPTION, - NO_EXTENTS_OPTION, - QUEUE_SIZE_OPTION, - REQUEST_SIZE_OPTION, - SYNCHRONOUS_OPTION, + OPT_HELP = CHAR_MAX + 1, + OPT_LONG_OPTIONS, + OPT_SHORT_OPTIONS, + OPT_ALLOCATED, + OPT_DESTINATION_IS_ZERO, + OPT_FLUSH, + OPT_NO_EXTENTS, + OPT_QUEUE_SIZE, + OPT_REQUEST_SIZE, + OPT_SYNCHRONOUS, }; const char *short_options = "C:pR:S:T:vV"; const struct option long_options[] = { - { "help", no_argument, NULL, HELP_OPTION }, - { "long-options", no_argument, NULL, LONG_OPTIONS_OPTION }, - { "allocated", no_argument, NULL, ALLOCATED_OPTION }, + { "help", no_argument, NULL, OPT_HELP }, + { "long-options", no_argument, NULL, OPT_LONG_OPTIONS }, + { "allocated", no_argument, NULL, OPT_ALLOCATED }, { "connections", required_argument, NULL, 'C' }, - { "destination-is-zero",no_argument, NULL, DESTINATION_IS_ZERO_OPTION }, - { "flush", no_argument, NULL, FLUSH_OPTION }, - { "no-extents", no_argument, NULL, NO_EXTENTS_OPTION }, + { "destination-is-zero",no_argument, NULL, OPT_DESTINATION_IS_ZERO }, + { "flush", no_argument, NULL, OPT_FLUSH }, + { "no-extents", no_argument, NULL, OPT_NO_EXTENTS }, { "progress", optional_argument, NULL, 'p' }, - { "queue-size", required_argument, NULL, QUEUE_SIZE_OPTION }, - { "request-size", required_argument, NULL, REQUEST_SIZE_OPTION }, + { "queue-size", required_argument, NULL, OPT_QUEUE_SIZE }, + { "request-size", required_argument, NULL, OPT_REQUEST_SIZE }, { "requests", required_argument, NULL, 'R' }, - { "short-options", no_argument, NULL, SHORT_OPTIONS_OPTION }, + { "short-options", no_argument, NULL, OPT_SHORT_OPTIONS }, { "sparse", required_argument, NULL, 'S' }, - { "synchronous", no_argument, NULL, SYNCHRONOUS_OPTION }, - { "target-is-zero", no_argument, NULL, DESTINATION_IS_ZERO_OPTION }, + { "synchronous", no_argument, NULL, OPT_SYNCHRONOUS }, + { "target-is-zero", no_argument, NULL, OPT_DESTINATION_IS_ZERO }, { "threads", required_argument, NULL, 'T' }, { "verbose", no_argument, NULL, 'v' }, { "version", no_argument, NULL, 'V' }, @@ -152,10 +152,10 @@ main (int argc, char *argv[]) break; switch (c) { - case HELP_OPTION: + case OPT_HELP: usage (stdout, EXIT_SUCCESS); - case LONG_OPTIONS_OPTION: + case OPT_LONG_OPTIONS: for (i = 0; long_options[i].name != NULL; ++i) { if (strcmp (long_options[i].name, "long-options") != 0 && strcmp (long_options[i].name, "short-options") != 0) @@ -163,30 +163,30 @@ main (int argc, char *argv[]) } exit (EXIT_SUCCESS); - case SHORT_OPTIONS_OPTION: + case OPT_SHORT_OPTIONS: for (i = 0; short_options[i]; ++i) { if (short_options[i] != ':' && short_options[i] != '+') printf ("-%c\n", short_options[i]); } exit (EXIT_SUCCESS); - case ALLOCATED_OPTION: + case OPT_ALLOCATED: allocated = true; break; - case DESTINATION_IS_ZERO_OPTION: + case OPT_DESTINATION_IS_ZERO: destination_is_zero = true; break; - case FLUSH_OPTION: + case OPT_FLUSH: flush = true; break; - case NO_EXTENTS_OPTION: + case OPT_NO_EXTENTS: extents = false; break; - case SYNCHRONOUS_OPTION: + case OPT_SYNCHRONOUS: synchronous = true; break; @@ -209,7 +209,7 @@ main (int argc, char *argv[]) } break; - case QUEUE_SIZE_OPTION: + case OPT_QUEUE_SIZE: if (sscanf (optarg, "%u", &queue_size) != 1) { fprintf (stderr, "%s: --queue-size: could not parse: %s\n", prog, optarg); @@ -217,7 +217,7 @@ main (int argc, char *argv[]) } break; - case REQUEST_SIZE_OPTION: + case OPT_REQUEST_SIZE: if (sscanf (optarg, "%u", &request_size) != 1) { fprintf (stderr, "%s: --request-size: could not parse: %s\n", prog, optarg);