Re: [PATCH 1/7] parallels: introduce and use string constants for network types and names

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 13.03.2015 16:52, Maxim Nestratov wrote:
> Signed-off-by: Maxim Nestratov <mnestratov@xxxxxxxxxxxxx>
> ---
>  src/parallels/parallels_network.c |    6 +++---
>  src/parallels/parallels_sdk.c     |    6 +++---
>  src/parallels/parallels_utils.h   |    8 +++++++-
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
> index 1d3b694..bb7ec5e 100644
> --- a/src/parallels/parallels_network.c
> +++ b/src/parallels/parallels_network.c
> @@ -211,12 +211,12 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
>          goto cleanup;
>      }
>  
> -    if (STREQ(tmp, "bridged")) {
> +    if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) {
>          def->forward.type = VIR_NETWORK_FORWARD_BRIDGE;
>  
>          if (parallelsGetBridgedNetInfo(def, jobj) < 0)
>              goto cleanup;
> -    } else if (STREQ(tmp, "host-only")) {
> +    } else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) {
>          def->forward.type = VIR_NETWORK_FORWARD_NONE;
>  

How about introducing 'int parallelsNetworkTypeFromString(const char *)'
using our ENUM_DECL and ENUM_IMPL macros? This code could then be turned
into:

    if ((def->forward.type = parallelNetworkTypeFromString(tmp)) < 0) {
        parallelsParseError();
        goto cleanup;
    }

    switch((virNetworkForwardType) def->forward.type) {
    case VIR_NETWORK_FORWARD_BRIDGE:
        if (parallelsGetBridgedNetInfo(def, jobj) < 0)
            goto cleanup;
        break;
    case VIR_NETWORK_FORWARD_NONE:
        if (parallelsGetHostOnlyNetInfo(def, def->name) < 0)
            goto cleanup;
        ...
    }

I find it more future proof then STREQ() spaghetti.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list