On 4/14/23 10:11, Richard W.M. Jones wrote: > 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 <ler...@redhat.com> >> --- >> 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.
Hmm. I didn't intend to do that. It seems that the following further files use the _OPTION suffix: dump/dump.c fuse/nbdfuse.c info/main.c ublk/nbdublk.c >From these, "dump/dump.c" and "info/main.c" don't exceed 80 chars, so I didn't plan to modify them at all. "fuse/nbdfuse.c" and "ublk/nbdublk.c" need wrapping, but not in their option tables. Renaming the options in these four other files, just for consistency's sake, seems overkill. Instead, I think we should pick a different approach for "copy/main.c". In the strictest sense, I only need to shorten DESTINATION_IS_ZERO_OPTION by three characters. Should I rename it to DEST_IS_ZERO_OPTION, or TARGET_IS_ZERO_OPTION? (The latter is better, because we already have a "--target-is-zero" long option, aliasing "--destination-is-zero".) So I'd replace patches #1 and #2 with that, and keep #3 and #4. Laszlo > > 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. > _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
