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

Shi Lei shilei.massclouds at gmx.com
Fri Jul 20 10:32:57 UTC 2018



On Fri, July 20, 2018 at 4:52 PM, Erik wrote:
> 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);

OK. 

> 
> ...
> 
> 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
> 

Thanks for your reviewing and directions, Erik!
And I have found examples you mentioned.
Have a nice day!

Shi Lei




More information about the libvir-list mailing list