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@xxxxxxx>
>
> ---
...
> + 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
