Re: MISRA violations in hypercall-defs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.08.2023 01:22, Stefano Stabellini wrote:
> On Tue, 8 Aug 2023, Jan Beulich wrote:
>> On 08.08.2023 10:47, Federico Serafini wrote:
>>> Hello everyone.
>>>
>>> I would like to to ask your opinion about the auto-generated file
>>> xen/include/xen/hypercall-defs.h which contains some violations of
>>> MISRA C:2012 Rule 8.3:
>>> "All declarations of an object or function shall use the same names and
>>> type qualifiers".
>>>
>>> Such violations can be seen at the following links
>>> (copy and paste the link on you browser, including also the characters
>>> after the '#'):
>>>
>>> - arm
>>> https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/origin/staging/ARM64-Set1/218/PROJECT.ecd;/by_service/MC3R1.R8.3.html#{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":true,"selector":{"enabled":true,"negated":false,"kind":2,"children":[{"enabled":true,"negated":false,"kind":0,"domain":"file","inputs":[{"enabled":true,"text":"xen/include/xen/hypercall-defs.h"}]}]}}}
>>>
>>> - x86
>>> https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/origin/staging/X86_64-Set1/218/PROJECT.ecd;/by_service/MC3R1.R8.3.html#{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":true,"selector":{"enabled":true,"negated":false,"kind":2,"children":[{"enabled":true,"negated":false,"kind":0,"domain":"file","inputs":[{"enabled":true,"text":"xen/include/xen/hypercall-defs.h"}]}]}}}
>>>
>>> Some of the violations are due to mismatches on the return types
>>> and the use of `ret_t`.
>>
>> We already said that ret_t will need deviating. For parameter names
>> it ought to be possible to suitably rename, like done elsewhere. Whether
>> that means renaming in the generator script or in the definitions likely
>> again needs judging on a case-by-case basis.
> 
> Is it the case that ret_t is purposedly defined as 'long' for 64-bit x86
> guests and 'int' for 32-bit x86 guests?

Yes.

> I am asking because it looks like we don't use ret_t at all on ARM and
> on the public interfaces.

And I wonder how you get away with this. Aiui hypercall return values are
32-bit on Arm32, so I'd expect you to be at risk of truncation issues in
a limited set of cases (see in particular the bottom of compat_memory_op(),
where out-of-range values are saturated rather than truncated). But maybe
in practice this can't happen?

Jan