Re: [XEN PATCH 4/5] x86/x86_emulate: change parameter name from 's' to 'state'

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.07.2023 00:49, Stefano Stabellini wrote:
> On Tue, 4 Jul 2023, Jan Beulich wrote:
>> On 29.06.2023 21:31, Stefano Stabellini wrote:
>>> On Thu, 29 Jun 2023, Federico Serafini wrote:
>>>> Change parameter name from 's' to 'state' in function definitions in
>>>> order to:
>>>> 1) keep consistency with the parameter names used in the corresponding
>>>>    declarations;
>>>> 2) keep consistency with parameter names used within x86_emulate.h;
>>>> 3) fix violations of MISRA C:2012 Rule 8.3.
>>>>
>>>> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
>>>
>>> You could use x86emul: as tag in the title. I'll let Jan choose the tag
>>> he prefers.
>>
>> x86emul: or x86/emul: is what we commonly use. That said, I don't like
>> this change. The files touched are pretty new, and it was deliberate
>> that I used s, not state, for the names. This is shorthand much like
>> (globally) we use v (instead of vcpu) and d (instead of domain).
> 
> Are you suggesting that the functions changed in this patch should be
> adapted in the other direction instead?  Meaning that the declaration is
> changed to match the definition instead of the opposite?
> 
> If so, are you referring to all the functions changed in this patch? Or
> only some?

All of the files touched here are ones which were recently introduced,
and which are deliberately the way they are. This "deliberately" really
goes as far as declarations and definitions disagreeing in names: For
the former, what matters are adjacent declarations in the header. For
the latter what matters is code readability. I'm sorry, I think the
Misra rule simply gets in the way of the original intentions here (and
I continue to disagree with there being any confusion from name
mismatches between declarations and definitions, the more that in the
case here it would be easy to avoid by simply omitting names in
declarations, but that is violating yet another rule I don't fully
agree with either, as voiced when discussing it).

My preferred course of action here would be to simply drop the patch.
The least bad adjustment, if one is absolutely necessary, would be to
change the declarations, but then in a way that all adjacent ones
remain consistent (which may in turn require some _other_ definitions
to change). The mid- to long-term goal certainly is to use "s" more
where "state" may be used right now.

Jan