[libvirt] [PATCH 10/41] remote: conditionalize IP socket usage in libvirtd daemon

Andrea Bolognani abologna at redhat.com
Fri Jul 26 13:27:19 UTC 2019


On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
[...]
> @@ -988,7 +1004,13 @@ int main(int argc, char **argv) {
>          int c;
>          char *tmp;
>  
> -        c = getopt_long(argc, argv, "ldf:p:t:vVh", opts, &optidx);
> +        c = getopt_long(argc, argv,
> +#ifdef ENABLE_IP
> +                        "ldf:p:t:vVh",
> +#else /* ! ENABLE_IP */
> +                        "df:p:t:vVh",
> +#endif /* ! ENABLE_IP */
> +                        opts, &optidx);

This looks pretty awful... Can you please do something like

  #ifdef ENABLE_IP
    const char *optstr = "ldf:p:t:vVh";
  #else /* ! ENABLE_IP */
    const char *optstr = "df:p:t:vVh";
  #endif /* ! ENABLE_IP */

  c = getopt_long(argc, argv, optstr, opts, &optidx);

instead?

[...]
> @@ -1003,9 +1025,11 @@ int main(int argc, char **argv) {
>          case 'd':
>              godaemon = 1;
>              break;

One extra empty line here for clarity, please...

> +#ifdef ENABLE_IP
>          case 'l':
>              ipsock = 1;
>              break;
> +#endif /* ! ENABLE_IP */

[...]
> +++ b/src/remote/remote_daemon_config.h
> @@ -41,21 +43,26 @@ struct daemonConfig {
>  
>      int auth_unix_rw;
>      int auth_unix_ro;

... and one here as well.


With the above addressed,

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>


[...]
> +    char **sasl_allowed_username_list;

I like this approach you've taken of completely eliminating structure
members when the corresponding feature is not compiled in, and in
fact I think we should use it more extensively: for example, we
should guard sasl_allowed_username_list with IF_SASL. Out of scope
for the patch series at hand, of course! :)

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list