Re: [dpdk-dev] [PATCH 1/2] net/virtio: do not re-enter clean up routines

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 11/2/18 4:19 PM, Chas Williams wrote:
> 
> 
> On 11/02/2018 10:33 AM, Ferruh Yigit wrote:
>> On 11/1/2018 2:45 PM, Luca Boccassi wrote:
>>> On Mon, 2017-07-17 at 19:05 -0400, Charles (Chas) Williams wrote:
>>>> .dev_uninit calls .dev_stop and .dev_close.  The work that is done in
>>>> those routines doesn't need repeated.  Use started and opened to
>>>> track
>>>> the adapter's status.
>>>>
>>>> Signed-off-by: Chas Williams <ciwillia@brocade.com>
>>
>> <...>
>>
>>>> @@ -253,7 +254,7 @@ struct virtio_hw {
>>>>       uint64_t    req_guest_features;
>>>>       uint64_t    guest_features;
>>>>       uint32_t    max_queue_pairs;
>>>> -    uint16_t    started;
>>>> +    bool        started;
>>>>       uint16_t    max_mtu;
>>>>       uint16_t    vtnet_hdr_size;
>>>>       uint8_t        vlan_strip;
>>>> @@ -268,6 +269,7 @@ struct virtio_hw {
>>>>       struct virtio_pci_common_cfg *common_cfg;
>>>>       struct virtio_net_config *dev_cfg;
>>>>       void        *virtio_user_dev;
>>>> +    bool        opened;
>>
>> This is already merged into next-net-virtio, but I would like to 
>> hightlight the
>> checkpatch warning about `bool` usage in struct [1].
>> Briefly it suggests preferring primitive data types against `bool` in 
>> structures
>> because its size is not clear.
>>
>> What do you think about it, do you have strong opinion to have them as 
>> bool?
> 
> Yes, I suppose I do.  bool is the "proper" representation for yes/no.
> The arguments cited aren't convincing.
> 
> The size of bool is the size of bool. The compiler gets to make that
> decision.  I don't get to decide the size of int either.  The size and
> alignemnt of bool should be optimal because your compiler probably knows
> more about the target than you do.  If your compiler can't figure out
> alignment in a structure, please fix the compiler.
> 
> bool is a primitive data type per the C99 standard.

I would like to keep it as bool too.

>> [1]
>> CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible
>> alignment issues - see: https://lkml.org/lkml/2017/11/21/384
>> #85: FILE: drivers/net/virtio/virtio_pci.h:234:
>> +       bool        started;
>>
>> CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible
>> alignment issues - see: https://lkml.org/lkml/2017/11/21/384
>> #93: FILE: drivers/net/virtio/virtio_pci.h:260:
>> +       bool        opened;
>>

BTW, I don't get this warning when running checkpatch, what kenrel
version is it coming from?

Thanks,
Maxime