On 19.12.2024 09:58, Nicola Vetrini wrote:
> On 2024-12-19 09:49, Jan Beulich wrote:
>> On 18.12.2024 15:25, Alessandro Zucchelli wrote:
>>> Rule 11.8 states as following: "A cast shall not remove any `const' or
>>> `volatile' qualification from the type pointed to by a pointer".
>>>
>>> Function `__hvm_copy' in `xen/arch/x86/hvm/hvm.c' is a double-use
>>> function, where the parameter needs to not be const because it can be
>>> set for write or not. As it was decided a new const-only function will
>>> lead to more developer confusion than it's worth, this violation is
>>> addressed by deviating the function.
>>> All cases of casting away const-ness are accompanied with a comment
>>> explaining why it is safe given the other flags passed in; such
>>> comment is used
>>> by the deviation in order to match the appropriate function call.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucche...@bugseng.com>
>>> ---
>
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -393,6 +393,12 @@ Fixing this violation would require to increase
>>> code complexity and lower readab
>>>
>>> -config=MC3R1.R11.8,reports+={safe,"any_area(any_loc(any_exp(macro(^container_of$))))"}
>>> -doc_end
>>>
>>> +-doc_begin="Function __hvm_copy in xen/arch/x86/hvm/hvm.c is a
>>> double-use
>>> +function, where the parameter needs to not be const because it can be
>>> set for
>>> +write or not"
>>> +-config=MC3A2.R11.8,reports+={safe,"any_area(any_loc(text(^.*__hvm_copy.*HVMCOPY_to_guest
>>>
>>> doesn't modify.*$)))"}
>>
>> This is probably good enough for now, yet still: It constrains
>> re-formatting
>> that we may want to do on such function calls. Personally I'd consider
>> it
>> entirely unexpected if I ended up (re)introducing a violation just by
>> re-
>> formatting one of those function calls to
>>
>> return __hvm_copy(
>> (void *)buf /* HVMCOPY_to_guest doesn't modify */,
>> addr, size, current, HVMCOPY_to_guest | HVMCOPY_linear,
>> PFEC_page_present | PFEC_write_access | pfec, pfinfo);
>>
>> yet aiui the pattern above would have this effect (I don't think .*
>> matches
>> newlines; instead I expect such regex-es to be applied to individual
>> lines
>> only). Thoughts anyone?
>
> we can simply drop the "__hvm_copy" part from the regex. The regex can
> be made multiline, or alternatively we can apply the search to a range
> of lines. By default it searches on the same location mentioned by the
> report, which in this case is the line containing __hvm_copy (range
> defaults to 0..0). However I would leave it either as is or without the
> __hvm_copy prefix.
Omitting the __hvm_copy part would again widen it too much for my taste.
Jan
