Re: [XEN PATCH 1/5] gnttab: address violation of MISRA C Rule 5.5

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 4 Jul 2025, Dmytro Prokopchuk1 wrote:
> Address a violation of MISRA C:2012 Rule 5.5:
> "Identifiers shall be distinct from macro names".
> 
> Reports for service MC3A2.R5.5:
> xen/common/grant_table.c: non-compliant macro 'update_gnttab_par'
> xen/common/grant_table.c: non-compliant macro 'parse_gnttab_limit'
> 
> The macros above are intended to discard function arguments (unused1, unused2)
> when compiling with different configurations of CONFIG_HYPFS.
> This can lead to confusion and unexpected behavior
> because the macro name and the function name are identical.
> Split the code and create two distinct function signatures.
> This ensures that the code behaves predictably and remains compliant.
> 
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>


I realize you tried to address Jan's comment about the global deviation.
In my opinion patch #2 and #3 are still OK, but I think this patch makes
things more confusing and error prone.

Can we find a way to deviate update_gnttab_par and parse_gnttab_limit
either with a SAF in-code comment (docs/misra/safe.json) or with a new
regex deviation (docs/misra/deviations.rst,
automation/eclair_analysis/ECLAIR/deviations.ecl)?


> ---
>  xen/common/grant_table.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index cf131c43a1..f3282a1d7b 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -126,18 +126,12 @@ static void __init cf_check 
> max_maptrack_frames_init(struct param_hypfs *par)
>      update_gnttab_par(opt_max_maptrack_frames, par,
>                        opt_max_maptrack_frames_val);
>  }
> -#else
> -#define update_gnttab_par(v, unused1, unused2)     update_gnttab_par(v)
> -#define parse_gnttab_limit(a, v, unused1, unused2) parse_gnttab_limit(a, v)
> -
> -static void update_gnttab_par(unsigned int val, struct param_hypfs *par,
> -                              char *parval)
> -{
> -}
> -#endif
>  
>  static int parse_gnttab_limit(const char *arg, unsigned int *valp,
>                                struct param_hypfs *par, char *parval)
> +#else
> +static int parse_gnttab_limit(const char *arg, unsigned int *valp)
> +#endif
>  {
>      const char *e;
>      unsigned long val;
> @@ -150,7 +144,9 @@ static int parse_gnttab_limit(const char *arg, unsigned 
> int *valp,
>          return -ERANGE;
>  
>      *valp = val;
> +#ifdef CONFIG_HYPFS
>      update_gnttab_par(val, par, parval);
> +#endif
>  
>      return 0;
>  }
> @@ -161,9 +157,13 @@ custom_runtime_param("gnttab_max_frames", 
> parse_gnttab_max_frames,
>  
>  static int cf_check parse_gnttab_max_frames(const char *arg)
>  {
> +#ifdef CONFIG_HYPFS
>      return parse_gnttab_limit(arg, &opt_max_grant_frames,
>                                param_2_parfs(parse_gnttab_max_frames),
>                                opt_max_grant_frames_val);
> +#else
> +    return parse_gnttab_limit(arg, &opt_max_grant_frames);
> +#endif
>  }
>  
>  static int cf_check parse_gnttab_max_maptrack_frames(const char *arg);
> @@ -173,9 +173,13 @@ custom_runtime_param("gnttab_max_maptrack_frames",
>  
>  static int cf_check parse_gnttab_max_maptrack_frames(const char *arg)
>  {
> +#ifdef CONFIG_HYPFS
>      return parse_gnttab_limit(arg, &opt_max_maptrack_frames,
>                                
> param_2_parfs(parse_gnttab_max_maptrack_frames),
>                                opt_max_maptrack_frames_val);
> +#else
> +    return parse_gnttab_limit(arg, &opt_max_maptrack_frames);
> +#endif
>  }
>  
>  #ifndef GNTTAB_MAX_VERSION
> -- 
> 2.43.0
>