[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] xen: Change parameter of generic_fls() to unsigned int
- To: Jan Beulich <jbeulich@xxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>
- From: Henry Wang <Henry.Wang@xxxxxxx>
- Date: Tue, 5 Sep 2023 01:25:05 +0000
- Accept-language: zh-CN, en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=xOw4Oi59PFX4ysVZJuIxorJytYFaH52BkmSQWBeFz7g=; b=DxA6D/INqXC5c6fyjbm/K/qOo0hfgn+NqYTd+UcAZ+mEbXiZL5TrovdoUcIDKYBtPbchPnzU6rHVqtOe0dyOn2QBX2+UCFj/WnNqSPPJ5upeAGPNzw8qve4Y070IaJb0glfsysBq3O2qrK+ZDi5J6LB9+SUyoPJISC7sw5WAyAHce4an/s1X29GZPkQNWcSLK07rD6HJjms2Y9asH7gxcK1AG9y6mUYyG8HMn3r2C6HozaQgs+Pr3OHibXIO5IBtkibd/q4FFptqkEQj2YS2pcpZ6v+rjT2vmmt07D4uWhi1ekc0IKI9jxTpe18os162rd7OkXoEwky8TXNeqcQfow==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=A4atISDtaPjEMaE5fWJdJYnkXlji1QuTj3a3WEYj75m3xhmy+QoviBJt2hscYmrThU5UapGQ1UW1hoe4hsBliL/zNZpKEV3eH6s7/huh/pYAXCAFT6/ZDsm/PdlzO0OlblPTWqSt1+P9Kl7OrECwvYYf/mcHy0y2pZ5wq6h+lX29rDylc6ieARVg1u1Ec4OdmgktPAcbrQsNWRpVq/KbAQEtW9BxmOnA1pYre6oamR1XNUssYaBp7wcQcV4tEzjVxqFKtjgw8FY0jtj4i8d8826iHnpFKCTTS3bPw8+7PjQtIm5rlfECWcjkBQd3QmLo41sDF3Xi0GHoziY7kFwzRg==
- Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
- Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Tue, 05 Sep 2023 01:26:17 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Nodisclaimer: true
- Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
- Thread-index: AQHZ3xBF/3psu2nmz0CVECL22ChB0bAKqSEAgAAKBICAABNjAIAAquSA
- Thread-topic: [PATCH 2/2] xen: Change parameter of generic_fls() to unsigned int
Hi Jan and Michal,
> On Sep 4, 2023, at 23:13, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> 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.
>From the release aspect, I am fine with doing the above-mentioned extra
changes, as we are not in the code freeze stage. From the development
point of view, I think whether doing such extra changes or not is supposed
to be an agreement between the developer and the maintainer.
Also this patch itself looks good to me so:
Reviewed-by: Henry Wang <Henry.Wang@xxxxxxx>
Kind regards,
Henry
>
>> - 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
>
|
