[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] tools: add offsetof() to xen-tools/libs.h
- To: Jan Beulich <jbeulich@xxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Mon, 27 Feb 2023 14:06:32 +0000
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=eDX2vW/r4HnF9lH5pLnNT+JaIccwT2rRSEbYnFJS6Jg=; b=U1yBqiGDy6Pwm7zQ/gMfDTyKE7Xcf6If0MGemavXsOEwRrBF/sKPSLJ8UFz2pzxsQRJlNSbFjkZa691ouMD8FA/UzGBTdi5QnhCvdtr1f99uk5yhxQ5OpDQQULoYcObxmlRkuwDAzA9lMhEdOZF6CFAnHW5P5Qi4xNZYKTlG9MecLHwHjJDQmlXbiTntAWdG/CXcEHi1ifdObO3CPLZofggyzZDka7wV4l21Ac7VkzdyrHk3bPK5eZaxj5nsHtUh5ZD0u47l3hSfV0JjmNfydL79ZWKS9DIsc1gzI0zL1SwOrZ4uH9VRXjPnC0iYHEGgndxDGoSvUqRJ/GJLoIE2Hg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CPOuiwOrdXJWyVDOviYSTosXzMZEkBj5K3Rgat78EjN497dHJseL1Xam8+ckgacBo3g4abzHPTxZiZPLf6Tb5gWWMst3YfpKlk1hHZ2itK3RKZOoMpSngT5RXRBak2qyIbyPr78+BG5FYs+ib6km8mtzNS0ZYf6Epphn086b35+96avuDNiB1ZRxN10jkigGRwybpn69zXehjmPVQD9AY4eDJZXlllRh0biyOERoOOTTeYqfLd5QF8+0db8jApWpQ1rr3vnsHBbIauHgenkd2Sdc0Qnmh/Q0l34Ec95CAHvY0QeRfBC+uug3rZNenIXVYsCSsny4+UChnVqGJ3BeKA==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Mon, 27 Feb 2023 14:07:25 +0000
- Ironport-data: A9a23:0MJlZK4sxP6ZSrEg8W5iwwxRtATGchMFZxGqfqrLsTDasY5as4F+v jFJWWmPPKmPazTzeop/bo239RlTsJTSx9M3SQFprHs8Hi5G8cbLO4+Ufxz6V8+wwm8vb2o8t plDNYOQRCwQZiWBzvt4GuG59RGQ7YnRGvynTraCYnsrLeNdYH9JoQp5nOIkiZJfj9G8Agec0 fv/uMSaM1K+s9JOGjt8B5mr9VU+7JwehBtC5gZlPaoR4weE/5UoJMl3yZ+ZfiOQrrZ8RoZWd 86bpJml82XQ+QsaC9/Nut4XpWVTH9Y+lSDX4pZnc/DKbipq/0Te4Y5iXBYoUm9Fii3hojxE4 I4lWapc6+seFvakdOw1C3G0GszlVEFM0OevzXOX6aR/w6BaGpdFLjoH4EweZOUlFuhL7W5ms vcaNCkUYy+6odmV27WZE+VH15sxM5y+VG8fkikIITDxK98DGMqGb4CUoNhS0XE3m9xEGuvYa 4wBcz1zYR/cYhpJfFAKFJY5m+TujX76G9FagAvN+exrvC6OnUooj+GF3Nn9I7RmQe18mEqCq 32A1GP+GhwAb/SUyCaf82LqjejK9c/+cNNCTOPpq64x6LGV7jMeAhk1U3SZmujniEifcM9DF UFM4zV7+MDe82TuFLERRSaQonSJoxodUNp4CPAh5UeGza+8ywSWHG8fVRZadccr8sQxQFQCy Vuhj97vQzt1v9W9WX+bs7uZsz62ESwUNnMZIz8JSxMf5Nvuq511iQjAJuuPC4awh9zxXD31n TaDqXFkg61J1JFSkaKm4VrAnjSg4IDTSRI47RnWWWTj6R5lYImiZMqj7l2zAet8Ebt1h2Kp5 BAs8/VyJshTVflhSATlrD0xIYyU
- Ironport-hdrordr: A9a23:BEsQsauZvrY00VU3RO+Yo/vT7skDstV00zEX/kB9WHVpm6yj+v xG/c5rsCMc7Qx6ZJhOo7+90cW7L080lqQFg7X5X43DYOCOggLBQL2KhbGI/9SKIVycygcy78 Zdm6gVMqyLMbB55/yKnTVRxbwbsaW6GKPDv5ag8590JzsaD52Jd21Ce36m+ksdfnggObMJUK Cyy+BgvDSadXEefq2AdwI4t7iqnaysqHr+CyR2fiIa1A==
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 27/02/2023 12:43 pm, Jan Beulich wrote:
> 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 ...
We still support 32bit builds of the toolstack on multiple
architectures, so I don't see bitness as an argument against.
I am slightly uneasy adding a header like this, but it really is only
common macros and I don't see how it could possibly change from that
given the existing uses. OTOH, removing things like the problematic
definition of offsetof() is an improvement.
I'd probably tentatively ack this patch, seeing as it is a minor
improvlement, but I think there's a middle option too. Rename libs.h to
macros.h or common-macros.h? IMO that would be something that's far
more obviously safe to include into firmware/, and something rather more
descriptive at all of its include sites.
Thoughts?
~Andrew
|
