Re: patch to fix short-option for certfile (-F) in nbd-client

[Eric Blake <eblake@redhat.com>]

Thanks for noticing the bug, and supplying a potential patch.  While
the subject line is accurate for 'what changed', git commits are
usually better if they also include a body stating 'why the change is
important'.  In this case, I'd add at a minimum:

Fixes: 18ceafed ("Add options to nbd-client for TLS support", v3.15)

or the longer:

When TLS support was added in commit 18ceafed, there were four long
options added, but only 3 short option aliases.  Fix the missing
intended alias of -F for --certfile.


I also don't know if Wouter has a strong policy on Signed-off-by:
attribution, but you may want to add one.

On Sat, Jul 22, 2023 at 10:48:08AM -0400, Matt Panaro wrote:
> --- a/nbd-client.c
> +++ b/nbd-client.c
> @@ -897,9 +900,9 @@ void disconnect(char* device) {
>  }
> 
>  #if HAVE_NETLINK
> -static const char *short_opts = "-A:B:b:c:C:d:gH:hK:LlnN:PpRSst:uVx";
> +static const char *short_opts = "-A:B:b:c:C:d:F:gH:hK:LlnN:PpRSst:uVx";
>  #else
> -static const char *short_opts = "-A:B:b:c:C:d:gH:hK:lnN:PpRSst:uVx";
> +static const char *short_opts = "-A:B:b:c:C:d:F:gH:hK:lnN:PpRSst:uVx";
>  #endif

Looking at 18ceafed, I'm wondering why we maintain two separate
short_opts lists in the first place.  It is just as easy to ALWAYS
accept the options, and have the options produce an error when they
are unsupported (see how F,K,A,H are handled in #ifdef WITH_GNUTLS
later on), as long as there is some easy way to probe whether a given
binary was compiled with or without gnutls and/or netlink support.

Another option might be to change the contents of short_opt (but at a
loss of alphabetical ordering), as in:

static const char *short_opts = "-"
#ifdef WITH_GNUTLS
  "A:F:H:K:"
#endif
#ifdef HAVE_NETLINK
  "L"
#endif
  "B:b:c:C:d:ghlnN:PpRSst:uVx";

where you can then avoid the #else branches later in the switch
statement for processing getopt results if the same conditionals are
added around the corresponding long options.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org