|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 09/14] ACPI: address a Misra rule 11.8 violation in Arm code
On 22.09.2026 23:18, Volodymyr Babchuk wrote: > Jan Beulich <jbeulich@xxxxxxxx> writes: >> On 22.09.2026 12:57, Volodymyr Babchuk wrote: >>> Jan Beulich <jbeulich@xxxxxxxx> writes: >>>> On 22.09.2026 03:38, Volodymyr Babchuk wrote: >>>>> Jan Beulich <jbeulich@xxxxxxxx> writes: >>>>>> Outside of drivers/acpi/tables/ (which is excluded from Eclair reporting >>>>>> for dubious reasons), arch/arm/acpi/domain_build.c is the only user of >>>>>> ACPI_COMPARE_NAME(). Make the macro const correct, at the expense of >>>>>> introducing a "const" variant of ACPI_CAST_PTR(), and at the expense of >>>>> >>>>> I think you can avoid adding ACPI_CAST_PTR() by providing a const >>>>> type. Like this: >>>>> >>>>> +#define ACPI_COMPARE_NAME(a, b) (*ACPI_CAST_PTR (const u32, a) >>>>> == \ >>>>> + *ACPI_CAST_PTR (const u32, b)) >>>> >>>> I don't think so. You did notice ... >>>> >>>>>> --- a/xen/include/acpi/acmacros.h >>>>>> +++ b/xen/include/acpi/acmacros.h >>>>>> @@ -103,6 +103,7 @@ >>>>>> * Pointer manipulation >>>>>> */ >>>>>> #define ACPI_CAST_PTR(t, p) ((t *) (acpi_uintptr_t) (p)) >>>>>> +#define ACPI_CAST_CPTR(t, p) ((const t *) (const >>>>>> acpi_uintptr_t) (p)) >>>> >>>> ... this, I assume. On the surface it looks odd, because one would expect >>>> acpi_uintptr_t to be a scalar type, like uintptr_t is. But it isn't: >>>> >>>> #ifndef acpi_uintptr_t >>>> #define acpi_uintptr_t void * >>>> #endif >>> >>> Well, that was unexpected. Talk about principle of least surprise... >>> >>>> The only alternative I see (somewhat more risky overall) would be to >>>> introduce >>>> >>>> #define acpi_uintptr_t uintptr_t >>> >>> What are the risks? I'd really prefer to have fewer gotchas in the code. >> >> It hides casting away of const-ness. Right here we would like that, yet in >> the general case I think we don't. Otoh Linux switched to the above in the >> 5.18 dev cycle. > > ... but for different reason. clang complained about pointer > subtraction with a null pointer. This does not fired in Xen because we > are not using ACPI_PTR_DIFF() yet. But, if we are going to use it, we'll > face the same problem. > > So, if we are already touching this parts, maybe it is better port that > Linux patch ([1]) and use const types as I suggested? > > > [1] > https://lore.kernel.org/linux-acpi/20210927121338.938994-1-arnd@xxxxxxxxxx, This has nothing to do with acpi_uintptr_t, and also looks to never have been committed. Were you instead meaning to refer to Linux commit ae57857b9b63? (Oddly enough that commit links the URL you quote above, just that that's not the patch which was committed.) If so, from a Misra perspective this has a significant downside: ACPI_CAST_PTR() could then be used to silently cast away constness. This, imo, is something we want to avoid when the goal is to make our code more safe. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |