Messages in this thread | | | Date | Thu, 8 Oct 2015 16:38:11 +0200 | Subject | Re: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info() | From | David Herrmann <> |
| |
Hi
On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote: > - Move `entry' and `owner' to the scope where they are used. Drop > redundand initialization of `entry'. Use conditional operator to set > the value of `owner'. > > - Set `ret' to zero right after call to kdbus_pool_slice_copy_kvec(), > not in the end of function. > > - Remove redundant goto. > > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net> > --- > ipc/kdbus/connection.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c > index b3c5f20a57d8..6a73ac3f444d 100644 > --- a/ipc/kdbus/connection.c > +++ b/ipc/kdbus/connection.c > @@ -1702,8 +1702,6 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp) > { > struct kdbus_meta_conn *conn_meta = NULL; > struct kdbus_pool_slice *slice = NULL; > - struct kdbus_name_entry *entry = NULL; > - struct kdbus_name_owner *owner = NULL; > struct kdbus_conn *owner_conn = NULL; > struct kdbus_item *meta_items = NULL; > struct kdbus_info info = {}; > @@ -1739,9 +1737,12 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp) > name = argv[1].item ? argv[1].item->str : NULL; > > if (name) { > + struct kdbus_name_entry *entry; > + struct kdbus_name_owner *owner; > + > entry = kdbus_name_lookup_unlocked(bus->name_registry, name); > - if (entry) > - owner = kdbus_name_get_owner(entry); > + owner = entry ? kdbus_name_get_owner(entry) : NULL; > +
This looks good to me.
> if (!owner || > !kdbus_conn_policy_see_name(conn, current_cred(), name) || > (cmd->id != 0 && owner->conn->id != cmd->id)) { > @@ -1804,17 +1805,14 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp) > ret = kdbus_pool_slice_copy_kvec(slice, 0, kvec, cnt, size); > if (ret < 0) > goto exit; > + ret = 0; > > kdbus_pool_slice_publish(slice, &cmd->offset, &cmd->info_size); > > if (kdbus_member_set_user(&cmd->offset, argp, typeof(*cmd), offset) || > kdbus_member_set_user(&cmd->info_size, argp, > - typeof(*cmd), info_size)) { > + typeof(*cmd), info_size)) > ret = -EFAULT; > - goto exit; > - } > - > - ret = 0;
Again, why? Now you have a random "ret = 0;" somewhere in between, instead of directly at the tail of the success-path.
Thanks David
> > exit: > up_read(&bus->name_registry->rwlock); > -- > 1.8.3.1 >
| |