Re: git: 07c64d74917e - main - acpica: Import ACPICA 20230628

From: Dmitry Salychev <dsl_at_FreeBSD.org>
Date: Thu, 01 Feb 2024 23:27:04 UTC
Jung-uk Kim <jkim@FreeBSD.org> writes:

> On 24. 2. 1., Dmitry Salychev wrote:
>> Hi,
>> Jung-uk Kim <jkim@FreeBSD.org> writes:
>> 
>>> On 24. 1. 31., Baptiste Daroussin wrote:
>>>> Hello,
>>>> Either this one or the previous import is breaking arm64 build
>>>> --- acpi_iort.o ---
>>>> /home/bapt/worktrees/main/sys/arm64/acpica/acpi_iort.c:103:4: error: field
>>>> 'data' with variable sized type 'union (unnamed union at
>>>> /home/bapt/worktrees/main/sys/arm64/acpica/acpi_iort.c:98:2
>>>> )' not at the end of a struct or class is a GNU extension
>>>> [-Werror,-Wgnu-variable-sized-type-not-at-end]
>>>>     103 |         } data;
>>>>           |           ^
>>>
>>> Sorry for the breakage.  I will fix it soon.
>>>
>>> BTW, this code was added by this:
>>>
>>> https://reviews.freebsd.org/D31267
>>>
>>> It seems struct iort_named_component was a hack, which duplicated
>>> ACPI_IORT_NAMED_COMPONENT but with a fixed length field
>>> DeviceName[32]. Is it really necessary?
>>>
>>> Jung-uk Kim
>> I'm struggling to understand (a) how the entire anonymous "data"
>> union
>> was added by https://reviews.freebsd.org/D31267 as was "named_comp"
>> only and (b) what the problem with the "struct iort_named_component" really
>> is as ACPI_IORT_ROOT_COMPLEX and ACPI_IORT_SMMU were re-defined as
>> incomplete types in the new version of ACPICA. Everything is compilable
>> as it used to be with the attached patch.
>
> FYI, ACPICA 20230331 dropped support for ANSI C (C89) and minimum
> requirement is C99 now.  One of the first feature they used was the
> variable-length array, which replaced "foo[1]" hack.  Previously,
> "sizeof(struct bar)" was not real size because of the (fake
> variable-length) array at the end of it.  Now they removed the hack
> and started using real variable-length arrays.  I believe the hack in
> acpi_iort.c was to work around the fake 1-byte array but it does not
> work any more because all ACPI_IORT_* are now using correct VLA.  So,
> the cleanest way to fix this is using ACPI_IORT_NAMED_COMPONENT
> instead of "struct iort_named_component" (see attached PoC patch),
> which also eliminates the need for DeviceName[32].
>
> Jung-uk Kim
>
> [2. text/x-patch; iort.diff]...

Please, open a review for your patch. I've several questions.

Regards,
Dmitry

-- 
https://wiki.freebsd.org/DmitrySalychev