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

On Tue, May 21, 2013 at 17:05:09 +0800, Guan Nan Ren wrote: > virConnectListAllInterfaces should support to list all of > interfaces when the value of flags is 0. The behaviour is > consistent with other virConnectListAll* APIs > --- > diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c > index cbba4fd..c6e069a 100644 > --- a/src/interface/interface_backend_netcf.c > +++ b/src/interface/interface_backend_netcf.c ... > @@ -361,16 +359,17 @@ netcfConnectListAllInterfaces(virConnectPtr conn, > /* XXX: Filter the result, need to be splitted once new filter flags > * except active|inactive are supported. > */ > - if (((status & NETCF_IFACE_ACTIVE) && > - (flags & VIR_CONNECT_LIST_INTERFACES_ACTIVE)) || > - ((status & NETCF_IFACE_INACTIVE) && > - (flags & VIR_CONNECT_LIST_INTERFACES_INACTIVE))) { > - if (ifaces) { > - iface_obj = virGetInterface(conn, ncf_if_name(iface), > - ncf_if_mac_string(iface)); > - tmp_iface_objs[niface_objs] = iface_obj; > - } > - niface_objs++; > + 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; IMHO this is much harder to read. I'd just rewrite the original condition as (i.e., !MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) is the part that was missing): 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))) Moreover, your version is wrong as it skips ncf_if_free(iface) in case iface should be skipped.