On 04.08.2023 16:00, Nicola Vetrini wrote:
>>
>> Upon further examination, I identified the following patterns:
>>
>> 1. Functions defined in .c called only from asm code (e.g., the
>> already mentioned __start_xen)
>> 2. Functions/variables declared in a .h, defined in a .c that does not
>> include the .h with the declaration
>> (e.g., 'fill_console_start_info' is defined in 'xen/drivers/vga.c',
>> declared in 'xen/include/xen/console.h' which is not visible when
>> compiling the .c).
>> 3. Variables that are either extern or not, such as 'acpi_gbl_FADT' in
>> 'xen/include/acpi/acglobal.h', depending on
>> DEFINE_ACPI_GLOBALS
>>
>> Below are the proposed resolution strategies:
>>
>> 1. I would advise to add the declaration in the relative .h, to
>> support automatic consistency checks with the
>> implementation and a quick reference when touching the asm.
>> 2. To comply with the rule, the header with the declaration should be
>> included. Also note that there are some
>> corner cases, such as 'get_sec', which is used in 'cper.h' without
>> including 'time.h' (which should gain a
>> declaration for it).
>> 3. One possible resolution pattern is including 'acglobal.h' twice
>> (either directly or indirectly trough acpi.h, if
>> the latter does not cause other issues) like so:
>>
>> (assuming DEFINE_ACPI_GLOBALS is undefined here)
>> #include "acglobal.h"
>> #define DEFINE_ACPI_GLOBALS
>> #include "acglobal.h"
>>
>> this way, the rule is followed properly, though it's not the
>> prettiest pattern and also clashes with the objectives
>> of D4.10 ("Precautions shall be taken in order to prevent the
>> contents of a header file being included
>> more than once"), but then a motivated exception is allowed there.
>
> One further question is whether functions under
> 'xen/common/coverage/gcov_base.c' should gain
> a declaration in 'gcov.h' or not, as they exist just for the purpose of
> being referenced
> by autogenerated profiling code. I see no reason why they shouldn't, but
> they can also be safely deviated,
> since they are not called by Xen code.
Imo it should be the compiler to provide a prototype for these (much
like it does for builtins), thus ensuring that an implementation
actually matches the compiler's expectations. Yet afaics it doesn't.
Jan
