Re: [XEN PATCH] xen/PCI: address violations of MISRA C:2012 Rules 8.2 and 8.3

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 11/09/23 10:42, Jan Beulich wrote:
> On 11.09.2023 10:15, Federico Serafini wrote:
> > Add missing parameter names and make function declarations and
> > definitions consistent. No functional change.
> >
> > Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> Since formally correct:
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> Nevertheless, it is probably about time to mention that ...
>
> > @@ -198,10 +198,11 @@ int pci_find_next_cap(u16 seg, u8 bus, unsigned int 
> > devfn, u8 pos, int cap);
> >   int pci_find_ext_capability(int seg, int bus, int devfn, int cap);
> >   int pci_find_next_ext_capability(int seg, int bus, int devfn, int start,
> >                                    int cap);
> > -const char *parse_pci(const char *, unsigned int *seg, unsigned int *bus,
> > -                      unsigned int *dev, unsigned int *func);
> > -const char *parse_pci_seg(const char *, unsigned int *seg, unsigned int *bus,
> > -                          unsigned int *dev, unsigned int *func, bool 
> > *def_seg);
> > +const char *parse_pci(const char *s, unsigned int *seg_p, unsigned int *bus_p,
> > +                      unsigned int *dev_p, unsigned int *func_p);
> > +const char *parse_pci_seg(const char *s, unsigned int *seg_p,
> > +                          unsigned int *bus_p, unsigned int *dev_p,
> > +                          unsigned int *func_p, bool *def_seg);
> ... parameter renaming like this, while fulfilling the word of Misra, is
> actually hampering readability. To someone wanting to use the function
> and hence looking at the declaration it is entirely uninteresting
> whether the _p suffixes are there; there were similar changes earlier on.
> The longer names merely make reading harder and - as is the case here -
> also may require a single declaration to wrap across more lines. I view
> such as going against the spirit of Misra (aiming to improve code
> clarity).
>
> Jan
I agree with you, but, the removal of _p would lead to
other (mechanical) changes to the function body.
In such cases, do you think that an improvement in readability
justifies the code churn?

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)