Re: [PATCH v3 for-4.14] pvcalls: Document correctly and explicitely the padding for all arches

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.06.2020 11:19, Julien Grall wrote:
> 
> 
> On 16/06/2020 09:26, Jan Beulich wrote:
>> On 13.06.2020 20:41, Julien Grall wrote:
>>> @@ -73,10 +76,18 @@ struct xen_pvcalls_request {
>>>               uint32_t flags;
>>>               grant_ref_t ref;
>>>               uint32_t evtchn;
>>> +#ifndef __i386__
>>> +            uint8_t pad[4];
>>> +#endif
>>
>> Where possible I think uint32_t would be slightly better to use.
> 
> OOI, why?

Because everything else here uses the wider type, plus the
question of why use a compound type (array) when a simple
one does.

>>
>>>           } connect;
>>>           struct xen_pvcalls_release {
>>>               uint64_t id;
>>>               uint8_t reuse;
>>> +#ifndef __i386__
>>> +            uint8_t pad[7];
>>> +#else
>>> +            uint8_t pad[3];
>>> +#endif
>>
>> For this I'd recommend uniform "uint8_t pad[3];" (i.e. outside
>> of any #ifdef) followed by a uint32_t again inside the #ifdef.
> 
> Same question here. The more the padding cannot be re-used.
> 
>>
>> Expressing everything through e.g. alignof() would seem even
>> better, but I can't currently think of a way to do so cleanly.
> 
> I am afraid I don't have time to look at how alignof() can work nicely. 
> Feel free to send a follow-up or pick-up the patch is you really want 
> alignof().

I didn't mean to ask that you find a solution. I merely pointed
out that expressing things properly rather than using hard coded
numbers would be nice.

Jan