lkml.org 
[lkml]   [2023]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: 回复: [PATCH v1] net:tipc:Remove rep eated initialization
From
Le 07/07/2023 à 03:24, 王明-软件底层技术部 a écrit :
> Hi CJ
> : )
> So what you're saying is there's no repeat initialization problem here.

Hi,

First, top posting is usually not the better way to answer email.


Anyway, in your patch, you have:
n = &grp->members.rb_node;
while (*n) {
tmp = container_of(*n, struct tipc_member, tree_node);
- parent = *n;
- tmp = container_of(parent, struct tipc_member, tree_node);
nkey = (u64)tmp->node << 32 | tmp->port;
if (key < nkey)
n = &(*n)->rb_left;

You are right, 'tmp' is initialized twice. It is even initialized twice,
with the same walue.

But in your patch, you also remove "parent = *n;"
So now, 'parent' is never updated in the function, and when
rb_link_node() is called after the while loop, it is NULL.

So your patch changed the behaviour of the code and looks broken.

Something like:
while (*n) {
tmp = container_of(*n, struct tipc_member, tree_node);
parent = *n;
- tmp = container_of(parent, struct tipc_member, tree_node);
nkey = (u64)tmp->node << 32 | tmp->port;
if (key < nkey)
n = &(*n)->rb_left;

would look good to me but, as said by Jakub in the thread, is really
unlikely to be applied.

The risk of breaking code (as you un-intentionally did) is higher than
the value of removing a redundant initialization.

Reviewing such patches also consume maintainers' time to check for such
potential errors and sometimes it is safer to leave things as-is in
order not to waste time and avoid potential troubles.

CJ


>
> -----邮件原件-----
> 发件人: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> 发送时间: 2023年7月7日 1:14
> 收件人: 王明-软件底层技术部 <machel@vivo.com>
> 抄送: Jon Maloy <jmaloy@redhat.com>; Ying Xue <ying.xue@windriver.com>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; tipc-discussion@lists.sourceforge.net; linux-kernel@vger.kernel.org; opensource.kernel <opensource.kernel@vivo.com>; Jakub Kicinski <kuba@kernel.org>
> 主题: Re: [PATCH v1] net:tipc:Remove repeated initialization
>
> [你通常不会收到来自 christophe.jaillet@wanadoo.fr 的电子邮件。请访问 https://aka.ms/LearnAboutSenderIdentification,以了解这一点为什么很重要]
>
> Le 06/07/2023 à 17:47, Jakub Kicinski a écrit :
>> On Thu, 6 Jul 2023 21:42:09 +0800 Wang Ming wrote:
>>> The original code initializes 'tmp' twice, which causes duplicate
>>> initialization issue.
>>> To fix this, we remove the second initialization of 'tmp' and use
>>> 'parent' directly forsubsequent operations.
>>>
>>> Signed-off-by: Wang Ming <machel@vivo.com>
>>
>> Please stop sending the "remove repeated initialization" patches to
>> networking, thanks.
>>
>>
>
> The patch also looks just bogus, as 'parent' is now always NULL when:
> rb_link_node(&m->tree_node, parent, n);
>
> is called after the while loop.
>
> CJ

\
 
 \ /
  Last update: 2023-07-07 07:26    [W:1.557 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site