On 10/8/14, 8:39 PM, "Eric Blake" <eblake@xxxxxxxxxx> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list