Impressive. :) ----- Original Message ----- From: "Eric Blake" <eblake@xxxxxxxxxx> To: "Guannan Ren" <gren@xxxxxxxxxx> Cc: libvir-list@xxxxxxxxxx Sent: Tuesday, May 21, 2013 10:49:57 PM Subject: Re: [PATCH] interface: list all interfaces with flags == 0 On 05/21/2013 06:49 AM, Guannan Ren wrote: Guannan's logic says when to drop: >>> + if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) && >>> + !((MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) && >>> + (status & NETCF_IFACE_ACTIVE)) || >>> + (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) && >>> + (status & NETCF_IFACE_INACTIVE)))) >>> + continue; Jirka's logic says when to keep; so it would need a ! around the overall expression if we want to turn it into the condition that represents when to drop. >> if (!MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) || >> (MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) && >> (status & NETCF_IFACE_ACTIVE)) || >> (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) && >> (status & NETCF_IFACE_INACTIVE))) > yes, currently, there are only one group of flags with two > values(active/inactive), > the way of yours is better to read. > if in future, there are more than one group of flags which are > going to support, > the way of my version will be better. Let's compare what happens for all four combinations of the two flags [active is flag 1, inactive is flag 2]: flags status Guannan's [0 means keep] Jirka's [1 means keep] 0 active 0&&!(0||0)=keep !0||0||0=keep 0 inactive 0&&!(0||0)=keep !0||0||0=keep 1 active 1&&!(1||0)=keep !1||1||0=keep 1 inactive 1&&!(0||0)=drop !1||0||0=drop 2 active 1&&!(0||0)=drop !1||0||0=drop 2 inactive 1&&!(0||1)=keep !1||0||1=keep 3 active 1&&!(1||0)=keep !1||1||0=keep 3 inactive 1&&!(0||1)=keep !1||0||1=keep Ultimately, the two expressions are equivalent (you can use deMorgan's law to prove the equivalence), but I find Jirka's positive logic a bit easier to reason with than Guannan's negative logic, even if Guannan's style copied from how we did it on other API. I do agree that for purposes of adding future filter groups, as well as minimizing nested conditionals, that the logic looks better in terms of drop checks: foreach item: if (filter group 1 drops) continue; if (filter group 2 drops) continue; item was kept by all filters, add it to return rather than logic in terms of keep checks: foreach item: if (filter group 1 keeps) if (filter group 2 keeps) add it to return -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list