[libvirt] [PATCH] replace 'if' type conditions with 'switch' for VIR_NETWORK_FORWARD_*

Erik Skultety eskultet at redhat.com
Fri Jul 20 08:52:30 UTC 2018


On Fri, Jul 20, 2018 at 12:00:53PM +0800, Shi Lei wrote:
> Hi, everyone!
> For VIR_NETWORK_FORWARD_*, I try to replace 'if' type conditions
> with typed 'switch()'.
> It might be more clear.
>
> Signed-off-by: Shi Lei <shilei.massclouds at gmx.com>
>
> ---

...

> +            networkRemoveFirewallRules(def);
> +            if (networkAddFirewallRules(def) < 0) {
> +                /* failed to add but already logged */
> +            }

^This if should go away, just call networkAddFirewallRules(def);

...

Conceptually, I agree with your changes, however, since you're already doing
the convert, recently we've changed our practice on how we approach the
switches. First, you should typecast def->forward.type to virNetworkForwardType
explicitly, so that we can utilize compile time checks. Second, we use the
default case only to report an out of range error with virReportEnumRangeError
(mainly because the values you care about are all already enumerated), you can
find plenty of examples throughout the code base if you look for the function I
just mentioned. No other objections from my side.

Regards,
Erik




More information about the libvir-list mailing list