Messages in this thread | | | Date | Fri, 9 Oct 2015 21:32:27 +0300 | From | Sergei Zviagintsev <> | Subject | Re: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast() |
| |
On Thu, Oct 08, 2015 at 04:34:27PM +0200, David Herrmann wrote: > Hi > > On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote: > > Do not initialize `name' and `ret' as values are assigned to them at > > the first use by kdbus_pin_dst(). Simplify handling of > > kdbus_conn_entry_insert() return value and drop useless goto. > > > > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net> > > --- > > ipc/kdbus/connection.c | 10 ++++------ > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c > > index db49f282a1bf..b3c5f20a57d8 100644 > > --- a/ipc/kdbus/connection.c > > +++ b/ipc/kdbus/connection.c > > @@ -1229,12 +1229,12 @@ static int kdbus_conn_unicast(struct kdbus_conn *src, > > struct kdbus_staging *staging) > > { > > const struct kdbus_msg *msg = staging->msg; > > - struct kdbus_name_entry *name = NULL; > > + struct kdbus_name_entry *name; > > struct kdbus_reply *wait = NULL; > > struct kdbus_conn *dst = NULL; > > struct kdbus_bus *bus = src->ep->bus; > > bool is_signal = (msg->flags & KDBUS_MSG_SIGNAL); > > - int ret = 0; > > + int ret; > > > > if (WARN_ON(msg->dst_id == KDBUS_DST_ID_BROADCAST) || > > WARN_ON(!(msg->flags & KDBUS_MSG_EXPECT_REPLY) && > > @@ -1245,7 +1245,6 @@ static int kdbus_conn_unicast(struct kdbus_conn *src, > > down_read(&bus->name_registry->rwlock); > > > > /* find and pin destination */ > > - > > If a comment is addressed to a whole following block, we usually put a > newline after it. Only if the comment is only addressed at the next > code-line, we don't.
Sorry, this change is here by mistake and shouldn't be in the patch at all.
> > > ret = kdbus_pin_dst(bus, staging, &name, &dst); > > if (ret < 0) > > goto exit; > > @@ -1276,11 +1275,10 @@ static int kdbus_conn_unicast(struct kdbus_conn *src, > > kdbus_bus_eavesdrop(bus, src, staging); > > > > ret = kdbus_conn_entry_insert(src, dst, staging, wait, name); > > - if (ret < 0 && !is_signal) > > - goto exit; > > > > /* signals are treated like broadcasts, recv-errors are ignored */ > > - ret = 0; > > + if (is_signal) > > + ret = 0; > > Why? Just to reduce the line-count? You break the code-flow here, by > making the success-path conditional, instead of the error-path.
IMO, it's easier to read as it's exacly what the comment says: ignore an error in the case of signal. But I don't mind omitting this change from the next submission.
> > Thanks > David > > > > > exit: > > up_read(&bus->name_registry->rwlock); > > -- > > 1.8.3.1 > >
| |