Re: [PATCH] radix-tree: don't left-shift negative values

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Nicola,

> On 13 Feb 2025, at 15:42, Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx> wrote:
> 
> On 2025-02-13 16:32, Nicola Vetrini wrote:
>> On 2025-02-13 16:01, Jan Beulich wrote:
>>> On 13.02.2025 15:52, Nicola Vetrini wrote:
>>>> On 2025-02-13 15:22, Jan Beulich wrote:
>>>>> Any (signed) integer is okay to pass into radix_tree_int_to_ptr(), yet
>>>>> left shifting negative values is UB. Use an unsigned intermediate type,
>>>>> reducing the impact to implementation defined behavior (for the
>>>>> unsigned->signed conversion).
>>>>> Also please Misra C:2012 rule 7.3 by dropping the lower case numeric
>>>>> 'l'
>>>>> tag.
>>>>> No difference in generated code, at least on x86.
>>>>> Fixes: b004883e29bb ("Simplify and build-fix (for some gcc versions)
>>>>> radix_tree_int_to_ptr()")
>>>>> Reported-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
>>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>> ---
>>>>> Bugseng: Why was the 7.3 violation not spotted by Eclair? According to
>>>>>         tagging.ecl the codebase is clean for this rule, aiui.
>>>> radix-tree.{c,h} is out of scope:
>>>> automation/eclair_analysis/ECLAIR/out_of_scope.ecl:32:-file_tag+={out_of_scope,"^xen/include/xen/radix-tree\\.h$"}
>>>> docs/misra/exclude-list.json:153:            "rel_path":
>>>> "common/radix-tree.c",
>>> Is there a record of why they are excluded? Is it further explainable
>>> why exclude-list.json mentions only the .c file and out_of_scope.ecl
>>> mentions only the .h one? Shouldn't different parts be in sync?
>> exclude-list.json is used to generate a configuration file for ECLAIR just 
>> before the analysis starts, so effectively both are excluded. It's a good 
>> point however to have only one file to handle exclusions, and use that file 
>> to generate the exclusion list dynamically, but then someone might want to 
>> exclude certain files only in some analyses and not others, which is not a 
>> good fit for exclude-list.json as it is now.
>> @Stefano, thoughts?
> 
> I forgot to address the first question: the (vague) reasons are listed in 
> exclude-list.json as the "comment" field; in most cases, it's because the 
> files have been imported from Linux, but the full rationale is something that 
> should be asked to the original author, which is Luca Fancellu.

So IIRC the full rationale is that since some files are imported from Linux, we 
would like to maintain them as they are
in order to ease backports. Misra fixes can be done, but they need to be 
upstreamed to Linux and backported to Xen.

Probably a re-evaluation could be done by the maintainers to see if some of 
these files could be removed from the exclusion
list.

Cheers,
Luca