Messages in this thread | | | Date | Fri, 7 Jul 2023 07:24:38 +0200 | Subject | Re: 回复: [PATCH v1] net:tipc:Remove rep eated initialization | From | Christophe JAILLET <> |
| |
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
| |