| Message ID | 310fe4a0ccd0ad45bcf1cd9811ac56c9a560dd0b.1710145041.git.simone.ballarin@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: address violation of MISRA C:2012 Directive 4.10 | expand |
On 11.03.2024 09:59, Simone Ballarin wrote: > Some headers, under specific circumstances (documented in a comment at > the beginning of the file), explicitly avoid inclusion guards: the caller > is responsible for including them correctly. > > These files are not supposed to comply with Directive 4.10: > "Precautions shall be taken in order to prevent the contents of a header > file being included more than once" > > This patch adds deviation cooments for headers that avoid guards. > > Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> > > --- > Changes in v3: > - fix inconsistent deviation ID > - change comment-based deviation text > Changes in v2: > - use the format introduced with doc/misra/safe.json instead of > a generic text-based deviation > --- > docs/misra/safe.json | 9 +++++++++ > xen/include/public/arch-x86/cpufeatureset.h | 1 + > xen/include/public/errno.h | 1 + > 3 files changed, 11 insertions(+) I understand something wants doing, but having such comments appear in public headers feels wrong to me. I'm afraid I have no good alternative suggestion. Jan
On Mon, 11 Mar 2024, Jan Beulich wrote: > On 11.03.2024 09:59, Simone Ballarin wrote: > > Some headers, under specific circumstances (documented in a comment at > > the beginning of the file), explicitly avoid inclusion guards: the caller > > is responsible for including them correctly. > > > > These files are not supposed to comply with Directive 4.10: > > "Precautions shall be taken in order to prevent the contents of a header > > file being included more than once" > > > > This patch adds deviation cooments for headers that avoid guards. > > > > Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> > > > > --- > > Changes in v3: > > - fix inconsistent deviation ID > > - change comment-based deviation text > > Changes in v2: > > - use the format introduced with doc/misra/safe.json instead of > > a generic text-based deviation > > --- > > docs/misra/safe.json | 9 +++++++++ > > xen/include/public/arch-x86/cpufeatureset.h | 1 + > > xen/include/public/errno.h | 1 + > > 3 files changed, 11 insertions(+) > > I understand something wants doing, but having such comments appear in public > headers feels wrong to me. I'm afraid I have no good alternative suggestion. Given that in both cases there is very nice explanation on how to use the headers as an in-code comment just above, could we embed the SAF-3-safe marker in the existing comment? If not, I think we should go with this patch as is (I don't think it is worth my, your, and Simone's time to look for alternatives).
On 14.03.2024 23:55, Stefano Stabellini wrote: > On Mon, 11 Mar 2024, Jan Beulich wrote: >> On 11.03.2024 09:59, Simone Ballarin wrote: >>> Some headers, under specific circumstances (documented in a comment at >>> the beginning of the file), explicitly avoid inclusion guards: the caller >>> is responsible for including them correctly. >>> >>> These files are not supposed to comply with Directive 4.10: >>> "Precautions shall be taken in order to prevent the contents of a header >>> file being included more than once" >>> >>> This patch adds deviation cooments for headers that avoid guards. >>> >>> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> >>> >>> --- >>> Changes in v3: >>> - fix inconsistent deviation ID >>> - change comment-based deviation text >>> Changes in v2: >>> - use the format introduced with doc/misra/safe.json instead of >>> a generic text-based deviation >>> --- >>> docs/misra/safe.json | 9 +++++++++ >>> xen/include/public/arch-x86/cpufeatureset.h | 1 + >>> xen/include/public/errno.h | 1 + >>> 3 files changed, 11 insertions(+) >> >> I understand something wants doing, but having such comments appear in public >> headers feels wrong to me. I'm afraid I have no good alternative suggestion. > > Given that in both cases there is very nice explanation on how to use > the headers as an in-code comment just above, could we embed the > SAF-3-safe marker in the existing comment? I'm afraid that won't address my remark, and I'm further afraid this would then render the SAF part of the comment ineffectual. > If not, I think we should go with this patch as is (I don't think it is > worth my, your, and Simone's time to look for alternatives). Easy alternative: Simply leave public headers alone. Jan
On Fri, 15 Mar 2024, Jan Beulich wrote: > On 14.03.2024 23:55, Stefano Stabellini wrote: > > On Mon, 11 Mar 2024, Jan Beulich wrote: > >> On 11.03.2024 09:59, Simone Ballarin wrote: > >>> Some headers, under specific circumstances (documented in a comment at > >>> the beginning of the file), explicitly avoid inclusion guards: the caller > >>> is responsible for including them correctly. > >>> > >>> These files are not supposed to comply with Directive 4.10: > >>> "Precautions shall be taken in order to prevent the contents of a header > >>> file being included more than once" > >>> > >>> This patch adds deviation cooments for headers that avoid guards. > >>> > >>> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> > >>> > >>> --- > >>> Changes in v3: > >>> - fix inconsistent deviation ID > >>> - change comment-based deviation text > >>> Changes in v2: > >>> - use the format introduced with doc/misra/safe.json instead of > >>> a generic text-based deviation > >>> --- > >>> docs/misra/safe.json | 9 +++++++++ > >>> xen/include/public/arch-x86/cpufeatureset.h | 1 + > >>> xen/include/public/errno.h | 1 + > >>> 3 files changed, 11 insertions(+) > >> > >> I understand something wants doing, but having such comments appear in public > >> headers feels wrong to me. I'm afraid I have no good alternative suggestion. > > > > Given that in both cases there is very nice explanation on how to use > > the headers as an in-code comment just above, could we embed the > > SAF-3-safe marker in the existing comment? > > I'm afraid that won't address my remark, and I'm further afraid this would > then render the SAF part of the comment ineffectual. > > > If not, I think we should go with this patch as is (I don't think it is > > worth my, your, and Simone's time to look for alternatives). > > Easy alternative: Simply leave public headers alone. I don't think it is a good idea to skip MISRA on public headers as they are used as part of the Xen build. I think adding the one line SAF comment is better. We still have strange tags like the following in public headers: * `incontents 150 evtchn Event Channels What does `incontents 150 even mean? If I grep for "incontents" I cannot find any explanation or definition. The SAF comment is easily greppable and with a clear definition. I am in favor of adding them (and removing "incontents" but that's a discussion for another day.)
diff --git a/docs/misra/safe.json b/docs/misra/safe.json index 952324f85c..e98956d604 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -28,6 +28,15 @@ }, { "id": "SAF-3-safe", + "analyser": { + "eclair": "MC3R1.D4.10" + }, + "name": "Dir 4.10: headers that leave it up to the caller to include them correctly", + "text": "Headers that deliberatively avoid inclusion guards explicitly leaving responsibility to the caller are allowed." + + }, + { + "id": "SAF-4-safe", "analyser": {}, "name": "Sentinel", "text": "Next ID to be used" diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index 0374cec3a2..f78a461d93 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -23,6 +23,7 @@ * their XEN_CPUFEATURE() being appropriate in the included context. */ +/* SAF-3-safe omitted inclusion guard */ #ifndef XEN_CPUFEATURE /* diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h index 5a78a7607c..28e064b67f 100644 --- a/xen/include/public/errno.h +++ b/xen/include/public/errno.h @@ -17,6 +17,7 @@ * will unilaterally #undef XEN_ERRNO(). */ +/* SAF-3-safe omitted inclusion guard */ #ifndef XEN_ERRNO /*
Some headers, under specific circumstances (documented in a comment at the beginning of the file), explicitly avoid inclusion guards: the caller is responsible for including them correctly. These files are not supposed to comply with Directive 4.10: "Precautions shall be taken in order to prevent the contents of a header file being included more than once" This patch adds deviation cooments for headers that avoid guards. Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> --- Changes in v3: - fix inconsistent deviation ID - change comment-based deviation text Changes in v2: - use the format introduced with doc/misra/safe.json instead of a generic text-based deviation --- docs/misra/safe.json | 9 +++++++++ xen/include/public/arch-x86/cpufeatureset.h | 1 + xen/include/public/errno.h | 1 + 3 files changed, 11 insertions(+)