On 10/8/14, 8:39 PM, "Eric Blake" <eblake@redhat.com> wrote:
On 10/08/2014 04:10 PM, Anirban Chakraborty wrote:
Needs rework after my comments on 1/2. I also wonder if this should just be folded in to that patch, and/or made into a switch statement where the compiler forces us to think about any future VIR_DOMAIN_NET_TYPE_* additions on whether they should return true or false.
bool virNetDevSupportBandwidth(virDomainNetType type) { switch (type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_DIRECT: case VIR_DOMAIN_NET_TYPE_ETHERNET: return true; case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: case VIR_DOMAIN_NET_TYPE_INTERNAL: case VIR_DOMAIN_NET_TYPE_HOSTDEV: case VIR_DOMAIN_NET_TYPE_LAST: /* cover all enums to appease the compiler */ ; } return false; }
We could have the function defined with a switch, however, instead of listing all the types, I could return false from the default case.
Something like: switch (type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_DIRECT: case VIR_DOMAIN_NET_TYPE_ETHERNET: return true;
default; return false;
Alas, using a default case means the compiler can no longer tell you about missed cases. I wrote my example to intentionally spell out ALL cases without a default, in order to ensure the compiler will loudly warn if we add another enum in the future.
I understood the intention. However, I was aiming for a compact code. If we add additional enums for network type, we should be adding those types in the above switch provided the new interface type supports bandwidth. I do not know if we should prevent future coding error by elaborating the current code. Forgive me, as I do not know the coding convention for libvirt. If this is the standard practice, I¹ll change the above switch as per your wish. Anirban