diff mbox series

[XEN,RFC] xen: avoid grep fodder define and undef

Message ID 0002f3df03aac283fa3dc7365e9040ab7ffaee2e.1710242950.git.federico.serafini@bugseng.com (mailing list archive)
State Superseded
Headers show
Series [XEN,RFC] xen: avoid grep fodder define and undef | expand

Commit Message

Federico Serafini March 12, 2024, 11:48 a.m. UTC
Place an "#if 0" before grep fodder definitions and remove the
"#undef".

This addresses several violations of MISRA C:2012 Rule 5.5
("Identifiers shall be distinct from macro names").

No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
Hello, I would like to know what you think about this change.
If you want to keep grep fodders as they are, an alternative could be to
deviate violations of Rule 5.5 involving the TYPE_SAFE macro.
---
 xen/include/xen/iommu.h    |  5 +----
 xen/include/xen/mm-frame.h | 15 +++------------
 2 files changed, 4 insertions(+), 16 deletions(-)

Comments

Jan Beulich March 12, 2024, 2:30 p.m. UTC | #1
On 12.03.2024 12:48, Federico Serafini wrote:
> Place an "#if 0" before grep fodder definitions and remove the
> "#undef".
> 
> This addresses several violations of MISRA C:2012 Rule 5.5
> ("Identifiers shall be distinct from macro names").
> 
> No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
> Hello, I would like to know what you think about this change.
> If you want to keep grep fodders as they are, an alternative could be to
> deviate violations of Rule 5.5 involving the TYPE_SAFE macro.

I think the original intention was to allow for some (arch?) override.
But we have lived without for long enough that I think we could do as
you suggest. One question though: Wasn't there also a rule against the
use of #undef, violations of which would then be eliminate here as well?
If so, I think that would want mentioning in the description. If not,
I'd like to give it a couple of days before ack-ing, just in case other
opinions would be voiced.

> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -33,13 +33,10 @@ TYPE_SAFE(uint64_t, dfn);
>  #define PRI_dfn     PRIx64
>  #define INVALID_DFN _dfn(~0ULL)
>  
> -#ifndef dfn_t
> +#if 0
>  #define dfn_t /* Grep fodder: dfn_t, _dfn() and dfn_x() are defined above */
>  #define _dfn
>  #define dfn_x
> -#undef dfn_t
> -#undef _dfn
> -#undef dfn_x
>  #endif

I have to admit that I'm a little surprised that Misra (Eclair) is
happy with this: The #define-s are all still there, after all.

Jan
Federico Serafini March 12, 2024, 7:54 p.m. UTC | #2
On 12/03/24 15:30, Jan Beulich wrote:
> On 12.03.2024 12:48, Federico Serafini wrote:
>> Place an "#if 0" before grep fodder definitions and remove the
>> "#undef".
>>
>> This addresses several violations of MISRA C:2012 Rule 5.5
>> ("Identifiers shall be distinct from macro names").
>>
>> No functional change.
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>> ---
>> Hello, I would like to know what you think about this change.
>> If you want to keep grep fodders as they are, an alternative could be to
>> deviate violations of Rule 5.5 involving the TYPE_SAFE macro.
> 
> I think the original intention was to allow for some (arch?) override.
> But we have lived without for long enough that I think we could do as
> you suggest. One question though: Wasn't there also a rule against the
> use of #undef, violations of which would then be eliminate here as well? > If so, I think that would want mentioning in the description. If not,
> I'd like to give it a couple of days before ack-ing, just in case other
> opinions would be voiced.

Yes, it is advisory Rule 20.5.

> 
>> --- a/xen/include/xen/iommu.h
>> +++ b/xen/include/xen/iommu.h
>> @@ -33,13 +33,10 @@ TYPE_SAFE(uint64_t, dfn);
>>   #define PRI_dfn     PRIx64
>>   #define INVALID_DFN _dfn(~0ULL)
>>   
>> -#ifndef dfn_t
>> +#if 0
>>   #define dfn_t /* Grep fodder: dfn_t, _dfn() and dfn_x() are defined above */
>>   #define _dfn
>>   #define dfn_x
>> -#undef dfn_t
>> -#undef _dfn
>> -#undef dfn_x
>>   #endif
> 
> I have to admit that I'm a little surprised that Misra (Eclair) is
> happy with this: The #define-s are all still there, after all.

The #define-s after the #if 0 are skipped by the preprocessor and this
is something that MISRA takes into account.

As an example, Directive 4.4 states that code should not be commented
out and explicitly cites the use of #if and #ifdef to exclude code from
the compilation.
diff mbox series

Patch

diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 9621459c63..ef57f31417 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -33,13 +33,10 @@  TYPE_SAFE(uint64_t, dfn);
 #define PRI_dfn     PRIx64
 #define INVALID_DFN _dfn(~0ULL)
 
-#ifndef dfn_t
+#if 0
 #define dfn_t /* Grep fodder: dfn_t, _dfn() and dfn_x() are defined above */
 #define _dfn
 #define dfn_x
-#undef dfn_t
-#undef _dfn
-#undef dfn_x
 #endif
 
 static inline dfn_t __must_check dfn_add(dfn_t dfn, unsigned long i)
diff --git a/xen/include/xen/mm-frame.h b/xen/include/xen/mm-frame.h
index c25e836f25..d973aec901 100644
--- a/xen/include/xen/mm-frame.h
+++ b/xen/include/xen/mm-frame.h
@@ -14,13 +14,10 @@  TYPE_SAFE(unsigned long, mfn);
  */
 #define INVALID_MFN_INITIALIZER { INVALID_MFN_RAW }
 
-#ifndef mfn_t
+#if 0
 #define mfn_t /* Grep fodder: mfn_t, _mfn() and mfn_x() are defined above */
 #define _mfn
 #define mfn_x
-#undef mfn_t
-#undef _mfn
-#undef mfn_x
 #endif
 
 static inline mfn_t __must_check mfn_add(mfn_t mfn, unsigned long i)
@@ -53,13 +50,10 @@  TYPE_SAFE(unsigned long, gfn);
  */
 #define INVALID_GFN_INITIALIZER { INVALID_GFN_RAW }
 
-#ifndef gfn_t
+#if 0
 #define gfn_t /* Grep fodder: gfn_t, _gfn() and gfn_x() are defined above */
 #define _gfn
 #define gfn_x
-#undef gfn_t
-#undef _gfn
-#undef gfn_x
 #endif
 
 static inline gfn_t __must_check gfn_add(gfn_t gfn, unsigned long i)
@@ -85,13 +79,10 @@  static inline bool gfn_eq(gfn_t x, gfn_t y)
 TYPE_SAFE(unsigned long, pfn);
 #define PRI_pfn          "05lx"
 
-#ifndef pfn_t
+#if 0
 #define pfn_t /* Grep fodder: pfn_t, _pfn() and pfn_x() are defined above */
 #define _pfn
 #define pfn_x
-#undef pfn_t
-#undef _pfn
-#undef pfn_x
 #endif
 
 #endif /* __XEN_FRAME_NUM_H__ */