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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.09.2023 17:13, Federico Serafini wrote:
> 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).
> 
> 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?

I didn't suggest changing the definition. I suggested keeping declaration
and definition (slightly) out of sync.

Jan