Re: [PATCH] xsm: misra rule 8.4 fix

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.12.2022 09:14, Jan Beulich wrote:
> On 07.12.2022 13:33, Michal Orzel wrote:
>> On 07/12/2022 03:12, Stefano Stabellini wrote:
>>> Fix two MISRA Issues Rule 8.4 ("A compatible declaration shall be
>>> visible when an object or function with external linkage is defined")
>>> found by cppcheck affecting xen/xsm/flask/ss/services.c.
>>>
>>> Fix the first issue by making policydb_loaded_version static and the
>>> second issue by declaring ss_initialized in a proper header.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
>>
>> cppcheck also reports findings for rule 8.4 with regards to the following 
>> functions:
>> - security_get_bools
>> - security_set_bools
>> - security_get_bool_value
>> - security_get_bool_name
>>
>> The prototypes for them are stored in xen/xsm/flask/include/conditional.h,
>> but services.c only does:
>> #include "conditional.h"
>>
>> This include refers to xen/xsm/flask/ss/conditional.h and not to 
>> xen/xsm/flask/include/conditional.h.
>> This means that we should also explicitly do:
>> #include <conditional.h>
> 
> And Misra has no rule disallowing such use of two different, identically
> named headers, for being potentially ambiguous?

Actually this is even more fragile than I thought when sending the above
question, albeit still working correctly as per gcc implementation defined
behavior. The further weakness stems from

CFLAGS-y += -iquote $(objtree)/xsm/flask/include
CFLAGS-y += -I$(srctree)/xsm/flask/include

which was added for out-of-tree builds, rendering in-tree builds search for
#include "..." files in xsm/flask/include too early. The only thing that
saves us is that the current directory is searched yet earlier.

However, as per gcc implementation defined behavior "#include <conditional.h>"
is likely wrong to use anyway, as this header can in no way be considered a
"system header"; it clearly falls under "header files of your own program",
where "own program" is Flask here. For being a system header the file ought
to live in include/xen/ or include/xsm/ (and accordingly be included via
"#include <xen/conditional.h>" or "#include <xsm/conditional.h>"), potentially
in a respective subdir there. My view is that these "#include <...>" (there
are more, albeit non-ambiguous ones) all want to be converted to
'#include "..."'. That'll then also eliminate the ambiguity with conditional.h
(as one will then [need to] come with a path prefix).

Jan