Re: [libvirt] [PATCH] interface: list all interfaces with flags == 0
Impressive. :)
----- Original Message -----
From: "Eric Blake" <eblake(a)redhat.com>
To: "Guannan Ren" <gren(a)redhat.com>
Cc: libvir-list(a)redhat.com
Sent: Tuesday, May 21, 2013 10:49:57 PM
Subject: Re: [libvirt] [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