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

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

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]