On 04/08/2023 02:35, Stefano Stabellini wrote:
I think that's OK for me. My only concern is that we should track the
project-wide deviations properly somewhere besides the ECLAIR
configuration under xen.git which is ECLAIR specific. So far we used
the
notes in docs/misra/rules.rst. I don't know if that sufficient, but we
could add a note for 8.4:
diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 8f0e4d3f25..5977bc9d5e 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -245,7 +245,8 @@ maintainers if you want to suggest a change.
- Required
- A compatible declaration shall be visible when an object or
function with external linkage is defined
- -
+ - No need for declarations when functions are only called from
+ assembly
* - `Rule 8.5
<https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_05_2.c>`_
- Required
On Thu, 3 Aug 2023, Nicola Vetrini wrote:
The headline of Rule 8.4 is as follows:
"A compatible declaration shall be visible when an object or
function with external linkage is defined".
Some functions reported in [1][2] are lacking a declaration in the
respective
header files;
as remarked on xen-devel's IRC channel, this is ok since they are only
called
from asm code (e.g., start_xen). A similar discussion
had taken place in the past (see [3]) and the general consensus was to
deviate
these cases.
If that is still the case, a suitable project-wide deviation can be
added to
address these violations.
[1]
https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/origin/staging/ARM64-Set1/210/PROJECT.ecd;/by_service/MC3R1.R8.4.html
[2]
https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/origin/staging/X86_64-Set1/210/PROJECT.ecd;/by_service/MC3R1.R8.4.html
[3]
https://lore.kernel.org/all/20220705210218.483854-2-burzalod...@gmail.com/
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.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
