From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933084AbbJISqn (ORCPT ); Fri, 9 Oct 2015 14:46:43 -0400 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:33597 "EHLO out5-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759129AbbJISql (ORCPT ); Fri, 9 Oct 2015 14:46:41 -0400 X-Sasl-enc: afI00PyUN/QYNGcMdcukKKEAsMqD8gmhwMOwpNMe+D+8 1444416390 Date: Fri, 9 Oct 2015 21:46:29 +0300 From: Sergei Zviagintsev To: David Herrmann Cc: Greg Kroah-Hartman , Daniel Mack , Djalal Harouni , linux-kernel Subject: Re: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst() Message-ID: <20151009184629.GK2189@localhost.localdomain> References: <461a280fc343eab2605a7ee2f08f193b334cc970.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 Hi, On Thu, Oct 08, 2015 at 04:40:33PM +0200, David Herrmann wrote: > Hi > > On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev wrote: > > - Reduce scope of `owner' var to the block where it is actually used. > > Use conditional operator to set its value. > > > > - Drop initialization of `dst' as it gets value later. > > > > - Drop separate exit point as we have only one use case of it. > > > > Signed-off-by: Sergei Zviagintsev > > --- > > ipc/kdbus/connection.c | 17 ++++++----------- > > 1 file changed, 6 insertions(+), 11 deletions(-) > > > > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c > > index 6a73ac3f444d..ace587ee951a 100644 > > --- a/ipc/kdbus/connection.c > > +++ b/ipc/kdbus/connection.c > > @@ -1048,10 +1048,8 @@ static int kdbus_pin_dst(struct kdbus_bus *bus, > > struct kdbus_conn **out_dst) > > { > > const struct kdbus_msg *msg = staging->msg; > > - struct kdbus_name_owner *owner = NULL; > > struct kdbus_name_entry *name = NULL; > > - struct kdbus_conn *dst = NULL; > > - int ret; > > + struct kdbus_conn *dst; > > > > lockdep_assert_held(&bus->name_registry->rwlock); > > > > @@ -1061,14 +1059,15 @@ static int kdbus_pin_dst(struct kdbus_bus *bus, > > return -ENXIO; > > > > if (!kdbus_conn_is_ordinary(dst)) { > > - ret = -ENXIO; > > - goto error; > > + kdbus_conn_unref(dst); > > + return -ENXIO; > > Looks good. > > > } > > } else { > > + struct kdbus_name_owner *owner; > > + > > I'd prefer if you avoid making this block-local. What is the rule of keeping (or not) things block-local? I always thought the less vars we keep in the mind at time, the easier is a piece of code to read. > > > name = kdbus_name_lookup_unlocked(bus->name_registry, > > staging->dst_name); > > - if (name) > > - owner = kdbus_name_get_owner(name); > > + owner = name ? kdbus_name_get_owner(name) : NULL; > > Looks good. > > > if (!owner) > > return -ESRCH; > > > > @@ -1094,10 +1093,6 @@ static int kdbus_pin_dst(struct kdbus_bus *bus, > > *out_name = name; > > *out_dst = dst; > > return 0; > > - > > -error: > > - kdbus_conn_unref(dst); > > - return ret; > > Looks good. > > Thanks > David > > > } > > > > static int kdbus_conn_reply(struct kdbus_conn *src, > > -- > > 1.8.3.1 > >