Re: [PATCH v12 for-xen-4.5 01/20] common/symbols: Export hypervisor symbols to privileged guest

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 09/29/2014 02:43 AM, Jan Beulich wrote:
>>>> On 26.09.14 at 18:49, <konrad.wilk@oracle.com> wrote:
>> On Fri, Sep 26, 2014 at 04:10:09PM +0100, Jan Beulich wrote:
>>>>>> On 26.09.14 at 16:58, <konrad.wilk@oracle.com> wrote:
>>>> If I move them just a bit:
>>>>
>>>>
>>>> diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
>>>> index 4f21b17..b97e476 100644
>>>> --- a/xen/include/public/platform.h
>>>> +++ b/xen/include/public/platform.h
>>>> @@ -538,9 +538,9 @@ struct xenpf_symdata {
>>>>                         /*      we reached the end                        */
>>>>   
>>>>       /* OUT variables */
>>>> -    char type;
>>>> -    XEN_GUEST_HANDLE(char) name;
>>>>       uint64_t address;
>>>> +    XEN_GUEST_HANDLE(char) name;
>>>> +    char type;
>>>>   };
>>>>   typedef struct xenpf_symdata xenpf_symdata_t;
>>>>   DEFINE_XEN_GUEST_HANDLE(xenpf_symdata_t);
>>>>
>>>>
>>>> 'pahole' is satisfied:
>>>>
>>>> struct xenpf_symdata {
>>   
>>>>      uint32_t                   namelen;              /*     0     4 */
>>   
>>>>      uint32_t                   symnum;               /*     4     4 */
>>   
>>>>      uint64_t                   address;              /*     8     8 */
>>   
>>>>      __guest_handle_char        name;                 /*    16     8 */
>>   
>>>>      char                       type;                 /*    24     1 */
>>   
>>>>                                                                              
>>   
>>>>      /* size: 32, cachelines: 1, members: 5 */
>>   
>>>>      /* padding: 7 */
>>   
>>>>      /* last cacheline: 32 bytes */
>>   
>>>> };
>>>>
>>>>
>>>> With that change, Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> This change buys us exactly nothing: Structure size doesn't change,
>>> and 7 bytes of padding are still there.
>> It does allow us to put more parameters (if we want to) at the end of the
>> structure instead of fitting them in between.
> Regardless of where the gap is, adding further fields in the future
> would work only if the code now checked that this field is zero
> (which first of all would require it being given a name). I keep
> pointing out that this should be done for all padding fields, but I'm
> afraid I may have missed doing so on this occasion.

I am not sure I understand how setting fields to zero would help with 
figuring out whether a new fields has been added. I can see how it can 
in some cases but not in general.

-boris