Messages in this thread | | | Date | Fri, 9 Oct 2015 21:46:29 +0300 | From | Sergei Zviagintsev <> | Subject | Re: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst() |
| |
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 <sergei@s15v.net> 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 <sergei@s15v.net> > > --- > > 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 > >
| |