[libvirt] [PATCH 38/41] remote: use enum helpers for parsing remote driver transport

[Andrea Bolognani]

On Tue, 2019-07-23 at 17:03 +0100, Daniel P. Berrangé wrote:
[...]
> +++ b/src/remote/remote_driver.c
> +VIR_ENUM_IMPL(remoteDriverTransport,
> +              REMOTE_DRIVER_TRANSPORT_LAST,
> +              "tls", "unix", "ssh", "libssh2", "ext", "tcp", "libssh");

One line per enum value, please.

[...]
> @@ -174,10 +192,17 @@ static int remoteSplitURIScheme(virURIPtr uri,
> +        p = *transport;
> +        while (*p) {
> +            *p = c_tolower(*p);
> +            p++;
> +        }

I can't believe we don't have a virString helper for this. Oh well.

[...]
> -            if (STRCASEEQ(transport_str, "tls")) {
> -                transport = trans_tls;
[...]
> -            } else if (STRCASEEQ(transport_str, "libssh")) {
> -                transport = trans_libssh;
> -            } else {
> -                virReportError(VIR_ERR_INVALID_ARG, "%s",
> -                               _("remote_open: transport in URL not recognised "
> -                                 "(should be tls|unix|ssh|ext|tcp|libssh2|libssh)"));
> +            if ((transport = remoteDriverTransportTypeFromString(transport_str)) < 0)
> +                return VIR_DRV_OPEN_ERROR;

You're no longer calling virReportError() when the user attempts to
use an unknown transport. While I don't think hardcoding the list of
valid transport in the error message is a good idea, neither is
failing without telling the user what they did wrong. Please restore
the virReportError() call.

[...]
> @@ -966,7 +966,7 @@ doRemoteOpen(virConnectPtr conn,
>      VIR_DEBUG("Connecting with transport %d", transport);
>      /* Connect to the remote service. */
>      switch (transport) {

Idea for a follow-up patch: "transport" could be cast to
remoteDriverTransport here, so that the compiler will ensure we're
covering all possible values.


Anyway, with the VIR_ENUM_IMPL() arguments formatted correctly and
some error message reported on unknown transport,

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

-- 
Andrea Bolognani / Red Hat / Virtualization