On 04.09.2023 16:03, Michal Orzel wrote:
> On 04/09/2023 15:28, Jan Beulich wrote:
>> On 04.09.2023 11:14, Michal Orzel wrote:
>>> --- a/xen/include/xen/bitops.h
>>> +++ b/xen/include/xen/bitops.h
>>> @@ -51,7 +51,7 @@ static inline int generic_ffs(int x)
>>> * fls: find last bit set.
>>> */
>>>
>>> -static __inline__ int generic_fls(int x)
>>> +static __inline__ int generic_fls(unsigned int x)
>>> {
>>> int r = 32;
>>>
>>
>> Even if perhaps not affected by UBSAN, generic_ffs() then wants taking care
>> of as well, imo. If additionally you switch __inline__ to inline, things
>> will become nicely symmetric with generic_f{f,l}sl().
> If others agree, I can switch generic_ffs() to take unsigned as well
> (together with s/__inline__/inline/).
> However, such change would no longer fall in fixes category (i.e. nothing
> wrong with ffs() taking int):
> - is it ok at this stage of release? (not sure but no problem for me to do
> this)
I'd say yes, but the release manager would have the ultimate say.
> - is it ok to mix a fix with non-fix change? (I always prefer fixes to be
> done as standalone changes)
Imo it is, to avoid introducing an inconsistency. While such may
not themselves be bugs, they increase the risk of introducing some.
Jan
