[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] interface: list all interfaces with flags == 0



Impressive. :)

----- Original Message -----
From: "Eric Blake" <eblake redhat com>
To: "Guannan Ren" <gren redhat com>
Cc: libvir-list 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]