Re: [PATCH 3/3] tools: add offsetof() to xen-tools/libs.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.02.2023 13:34, Juergen Gross wrote:
> On 27.02.23 13:31, Jan Beulich wrote:
>> On 27.02.2023 13:09, Juergen Gross wrote:
>>> --- a/tools/firmware/hvmloader/util.h
>>> +++ b/tools/firmware/hvmloader/util.h
>>> @@ -30,9 +30,6 @@ enum {
>>>   #define SEL_DATA32          0x0020
>>>   #define SEL_CODE64          0x0028
>>>   
>>> -#undef offsetof
>>> -#define offsetof(t, m) ((unsigned long)&((t *)0)->m)
>>> -
>>>   #undef NULL
>>>   #define NULL ((void*)0)
>>>   
>>> diff --git a/tools/firmware/include/stddef.h 
>>> b/tools/firmware/include/stddef.h
>>> index c7f974608a..926ae12fa5 100644
>>> --- a/tools/firmware/include/stddef.h
>>> +++ b/tools/firmware/include/stddef.h
>>> @@ -1,10 +1,10 @@
>>>   #ifndef _STDDEF_H_
>>>   #define _STDDEF_H_
>>>   
>>> +#include <xen-tools/libs.h>
>>
>> I'm not convinced firmware/ files can validly include this header.
> 
> This file only contains macros which don't call any external functions.
> 
> Would you be fine with me adding a related comment to it?

If it was to be usable like this, that would be a prereq, but personally
I don't view this as sufficient. The environment code runs in that lives
under firmware/ is simply different (hvmloader, for example, is 32-bit
no matter that the toolstack would normally be 64-bit), so to me it
feels like setting up a trap for ourselves. If Andrew or Roger are fully
convinced this is a good move, I won't be standing in the way ...

Jan