From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755491AbbJISPy (ORCPT ); Fri, 9 Oct 2015 14:15:54 -0400 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:47124 "EHLO out5-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754924AbbJISPx (ORCPT ); Fri, 9 Oct 2015 14:15:53 -0400 X-Sasl-enc: DlAljZVSTZIyBzc61/DPg5/rFnK7lmi2bU9JAFOWBISB 1444414552 Date: Fri, 9 Oct 2015 21:15:51 +0300 From: Sergei Zviagintsev To: David Herrmann Cc: Greg Kroah-Hartman , Daniel Mack , David Herrmann , Djalal Harouni , linux-kernel Subject: Re: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call() Message-ID: <20151009181551.GH2189@localhost.localdomain> References: <8cc42b4b6f50de0ce700d4c3b18e732617469a1d.1444302968.git.sergei@s15v.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 08, 2015 at 04:32:47PM +0200, David Herrmann wrote: > Hi > > On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev wrote: > > Do not initialize `wait' and `name' as values are assigned to them at > > first use: `wait' gets its value from kdbus_reply_find(), `name' is set > > by kdbus_pin_dst(). > > > > Remove redundant code. goto isn't required as we reached exit point > > already. Setting `ret' to zero is unnecessary because > > kdbus_conn_entry_insert() returns 0 on success. > > > > Signed-off-by: Sergei Zviagintsev > > --- > > ipc/kdbus/connection.c | 10 ++-------- > > 1 file changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c > > index a4d7414ecaea..db49f282a1bf 100644 > > --- a/ipc/kdbus/connection.c > > +++ b/ipc/kdbus/connection.c > > @@ -1159,8 +1159,8 @@ static struct kdbus_reply *kdbus_conn_call(struct kdbus_conn *src, > > ktime_t exp) > > { > > const struct kdbus_msg *msg = staging->msg; > > - struct kdbus_name_entry *name = NULL; > > - struct kdbus_reply *wait = NULL; > > + struct kdbus_name_entry *name; > > + struct kdbus_reply *wait; > > struct kdbus_conn *dst = NULL; > > struct kdbus_bus *bus = src->ep->bus; > > int ret; > > @@ -1212,14 +1212,8 @@ static struct kdbus_reply *kdbus_conn_call(struct kdbus_conn *src, > > } > > > > /* send message */ > > - > > kdbus_bus_eavesdrop(bus, src, staging); > > - > > ret = kdbus_conn_entry_insert(src, dst, staging, wait, name); > > - if (ret < 0) > > - goto exit; > > - > > - ret = 0; > > Who says kdbus_conn_entry_insert() returns 0? It might be >0. I'd > prefer the explicit check. That is clearly written in its kernel-doc and its code. In this particular case 'ret > 0' situation doesn't matter at all as we only do 'ret < 0' test latter and return `wait' var (the commit message isn't clear about that). > > Thanks > David > > > > > exit: > > up_read(&bus->name_registry->rwlock); > > -- > > 1.8.3.1 > >