Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1242233 > unrolled thread

[PATCH 00/44] kdbus cleanups

Started bySergei Zviagintsev <sergei@s15v.net>
First post2015-10-08 13:40 +0200
Last post2015-10-09 09:30 +0200
Articles 20 on this page of 61 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/44] kdbus cleanups Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 32/44] kdbus: Remove duplicated code from kdbus_conn_lock2() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 16/44] kdbus: Drop redundant code from kdbus_name_acquire() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 42/44] kdbus: Check if fd is allocated before trying to free it Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 37/44] kdbus: Fix error path in kdbus_meta_proc_collect_cgroup() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 34/44] kdbus: Improve kdbus_conn_entry_sync_attach() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 39/44] kdbus: Cleanup kdbus_user_lookup() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 10/44] kdbus: Use conditional operator Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 01/44] Documentation/kdbus: Document new name registry flags Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:40 +0200
        Re: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:40 +0200
    [PATCH 12/44] kdbus: Use conventional list macros in __kdbus_pool_slice_release() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:30 +0200
        Re: [PATCH 15/44] kdbus: Simplify bitwise expression in  kdbus_meta_get_mask() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:00 +0200
    [PATCH 08/44] kdbus: Rename var in kdbus_meta_export_caps() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 41/44] kdbus: Fix memfd install algorithm Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 27/44] kdbus: Cleanup kdbus_conn_new() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 20/44] kdbus: Drop useless initialization from kdbus_cmd_hello() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 22/44] kdbus: Cleanup error path in kdbus_staging_new_user() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 05/44] kdbus: Add comment on merging free pool slices Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 05/44] kdbus: Add comment on merging free pool slices David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:00 +0200
        Re: [PATCH 05/44] kdbus: Add comment on merging free pool slices Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:20 +0200
    [PATCH 02/44] uapi: kdbus.h: Kernel-doc fixes Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 02/44] uapi: kdbus.h: Kernel-doc fixes David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 15:50 +0200
    [PATCH 30/44] kdbus: Cleanup kdbus_meta_proc_mask() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 30/44] kdbus: Cleanup kdbus_meta_proc_mask() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:50 +0200
    [PATCH 14/44] kdbus: Simplify expression in kdbus_get_memfd() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 14/44] kdbus: Simplify expression in kdbus_get_memfd() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:30 +0200
    [PATCH 13/44] kdbus: Use list_next_entry() in kdbus_queue_entry_unlink() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 13/44] kdbus: Use list_next_entry() in kdbus_queue_entry_unlink() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:10 +0200
    [PATCH 33/44] kdbus: Improve kdbus_staging_reserve() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 35/44] kdbus: Drop goto from kdbus_queue_entry_link() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 17:00 +0200
        Re: [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:50 +0200
    [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:40 +0200
        Re: [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:10 +0200
    [PATCH 11/44] kdbus: Cosmetic fix of kdbus_name_is_valid() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    [PATCH 23/44] kdbus: Cleanup kdbus_conn_call() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:40 +0200
        Re: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:20 +0200
    [PATCH 03/44] kdbus: Kernel-docs and comments trivial fixes Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 03/44] kdbus: Kernel-docs and comments trivial fixes David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 15:50 +0200
    [PATCH 28/44] kdbus: Cleanup kdbus_queue_entry_new() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:40 +0200
        Re: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:50 +0200
    [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:50 +0200
        Re: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:50 +0200
    [PATCH 09/44] kdbus: Remove unused KDBUS_MSG_MAX_SIZE constant Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    [PATCH 19/44] kdbus: Drop useless initialization from kdbus_conn_reply() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:30 +0200
        Re: [PATCH 18/44] kdbus: Add var initialization to  kdbus_conn_entry_insert() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:00 +0200
    [PATCH 06/44] kdbus: Fix kernel-doc for struct kdbus_gaps Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    Re: [PATCH 00/44] kdbus cleanups David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 17:30 +0200
      Re: [PATCH 00/44] kdbus cleanups Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 09:30 +0200

Page 3 of 4 — ← Prev page 1 2 [3] 4  Next page →

#1242266 — [PATCH 11/44] kdbus: Cosmetic fix of kdbus_name_is_valid()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 11/44] kdbus: Cosmetic fix of kdbus_name_is_valid()
Message-ID<qhhRU-2oG-23@gated-at.bofh.it>
In reply to#1242233
Initialize `dot' during declaration but not in the for-loop, as it is
not used as a loop cursor.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/names.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index 6b31b38ac2ad..4dea83defc8e 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -530,10 +530,10 @@ void kdbus_name_release_all(struct kdbus_name_registry *reg,
  */
 bool kdbus_name_is_valid(const char *p, bool allow_wildcard)
 {
-	bool dot, found_dot = false;
+	bool dot = true, found_dot = false;
 	const char *q;
 
-	for (dot = true, q = p; *q; q++) {
+	for (q = p; *q; q++) {
 		if (*q == '.') {
 			if (dot)
 				return false;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242267 — [PATCH 23/44] kdbus: Cleanup kdbus_conn_call()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 23/44] kdbus: Cleanup kdbus_conn_call()
Message-ID<qhhRU-2oG-29@gated-at.bofh.it>
In reply to#1242233
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 <sergei@s15v.net>
---
 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;
 
 exit:
 	up_read(&bus->name_registry->rwlock);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242439 — Re: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:40 +0200
SubjectRe: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call()
Message-ID<qhkwq-6iy-29@gated-at.bofh.it>
In reply to#1242267
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> 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 <sergei@s15v.net>
> ---
>  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.

Thanks
David

>
>  exit:
>         up_read(&bus->name_registry->rwlock);
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243577 — Re: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-09 20:20 +0200
SubjectRe: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call()
Message-ID<qhKqR-1vd-3@gated-at.bofh.it>
In reply to#1242439
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 <sergei@s15v.net> 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 <sergei@s15v.net>
> > ---
> >  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
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242268 — [PATCH 03/44] kdbus: Kernel-docs and comments trivial fixes

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 03/44] kdbus: Kernel-docs and comments trivial fixes
Message-ID<qhhRU-2oG-33@gated-at.bofh.it>
In reply to#1242233
 - kdbus_conn_unref(): Fix style issue to stay similar to other
   kernel-docs.

 - kdbus_conn_disconnect(): Update the name of parameter:
   "ensure_msg_list_empty" -> "ensure_queue_empty".

 - kdbus_conn_entry_insert(): Fix typo: dot -> comma.

 - struct kdbus_conn: Remove lost "activator for" string.

 - kdbus_fs_init(): Fix typo: "nameing" -> "naming".

 - kdbus_node_deactivate(): Fix typo in comment:
   "node as children" -> "node has children".

 - kdbus_pool_slice_alloc(): Remove description of kvec and iovec as
   they relate to the old code.

 - struct kdbus_queue_entry: Fix typo: "messages" -> "message".

 - kdbus_pool_slice_publish(): Remove obsolete line from the
   description.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/connection.c | 6 +++---
 ipc/kdbus/connection.h | 1 -
 ipc/kdbus/fs.c         | 2 +-
 ipc/kdbus/node.c       | 4 ++--
 ipc/kdbus/pool.c       | 5 +----
 ipc/kdbus/queue.h      | 2 +-
 6 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index ef63d6533273..4f3cd370ecd9 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -317,7 +317,7 @@ struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn)
 
 /**
  * kdbus_conn_unref() - drop a connection reference
- * @conn:		Connection (may be NULL)
+ * @conn:		Connection, may be %NULL
  *
  * When the last reference is dropped, the connection's internal structure
  * is freed.
@@ -490,7 +490,7 @@ exit_disconnect:
  *			case the connection's message list is not
  *			empty
  *
- * If @ensure_msg_list_empty is true, and the connection has pending messages,
+ * If @ensure_queue_empty is true, and the connection has pending messages,
  * -EBUSY is returned.
  *
  * Return: 0 on success, negative errno on failure
@@ -880,7 +880,7 @@ static int kdbus_conn_entry_sync_attach(struct kdbus_conn *conn_dst,
  * @reply:		The reply tracker to attach to the queue entry
  * @name:		Destination name this msg is sent to, or NULL
  *
- * Return: 0 on success. negative error otherwise.
+ * Return: 0 on success, negative error otherwise.
  */
 int kdbus_conn_entry_insert(struct kdbus_conn *conn_src,
 			    struct kdbus_conn *conn_dst,
diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
index 1ad082014faa..679f393d3e68 100644
--- a/ipc/kdbus/connection.h
+++ b/ipc/kdbus/connection.h
@@ -53,7 +53,6 @@ struct kdbus_staging;
  * @reply_list:		List of connections this connection should
  *			reply to
  * @work:		Delayed work to handle timeouts
- *			activator for
  * @match_db:		Subscription filter to broadcast messages
  * @meta_proc:		Process metadata of connection creator, or NULL
  * @meta_fake:		Faked metadata, or NULL
diff --git a/ipc/kdbus/fs.c b/ipc/kdbus/fs.c
index 09c480924b9e..47e652f2ee0e 100644
--- a/ipc/kdbus/fs.c
+++ b/ipc/kdbus/fs.c
@@ -383,7 +383,7 @@ static struct file_system_type fs_type = {
  * kdbus_fs_init() - register kdbus filesystem
  *
  * This registers a filesystem with the VFS layer. The filesystem is called
- * `KBUILD_MODNAME "fs"', which usually resolves to `kdbusfs'. The nameing
+ * `KBUILD_MODNAME "fs"', which usually resolves to `kdbusfs'. The naming
  * scheme allows to set KBUILD_MODNAME to "kdbus2" and you will get an
  * independent filesystem for developers.
  *
diff --git a/ipc/kdbus/node.c b/ipc/kdbus/node.c
index 89f58bc85433..133fb01a35cc 100644
--- a/ipc/kdbus/node.c
+++ b/ipc/kdbus/node.c
@@ -557,8 +557,8 @@ void kdbus_node_deactivate(struct kdbus_node *node)
 	/*
 	 * To avoid recursion, we perform back-tracking while deactivating
 	 * nodes. For each node we enter, we first mark the active-counter as
-	 * deactivated by adding BIAS. If the node as children, we set the first
-	 * child as current position and start over. If the node has no
+	 * deactivated by adding BIAS. If the node has children, we set the
+	 * first child as current position and start over. If the node has no
 	 * children, we drain the node by waiting for all active refs to be
 	 * dropped and then releasing the node.
 	 *
diff --git a/ipc/kdbus/pool.c b/ipc/kdbus/pool.c
index 63ccd55713c7..0433e26b777e 100644
--- a/ipc/kdbus/pool.c
+++ b/ipc/kdbus/pool.c
@@ -192,9 +192,7 @@ static struct kdbus_pool_slice *kdbus_pool_find_slice(struct kdbus_pool *pool,
  * @accounted:	Whether this slice should be accounted for
  *
  * The returned slice is used for kdbus_pool_slice_release() to
- * free the allocated memory. If either @kvec or @iovec is non-NULL, the data
- * will be copied from kernel or userspace memory into the new slice at
- * offset 0.
+ * free the allocated memory.
  *
  * Return: the allocated slice on success, ERR_PTR on failure.
  */
@@ -411,7 +409,6 @@ void kdbus_pool_publish_empty(struct kdbus_pool *pool, u64 *off, u64 *size)
  * This prepares a slice to be published to user-space.
  *
  * This call combines the following operations:
- *   * the memory region is flushed so the user's memory view is consistent
  *   * the slice is marked as referenced by user-space, so user-space has to
  *     call KDBUS_CMD_FREE to release it
  *   * the offset and size of the slice are written to the given output
diff --git a/ipc/kdbus/queue.h b/ipc/kdbus/queue.h
index bf686d182ce1..92f7549d8c9b 100644
--- a/ipc/kdbus/queue.h
+++ b/ipc/kdbus/queue.h
@@ -38,7 +38,7 @@ struct kdbus_queue {
 };
 
 /**
- * struct kdbus_queue_entry - messages waiting to be read
+ * struct kdbus_queue_entry - message waiting to be read
  * @entry:		Entry in the connection's list
  * @prio_node:		Entry in the priority queue tree
  * @prio_entry:		Queue tree node entry in the list of one priority
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242371 — Re: [PATCH 03/44] kdbus: Kernel-docs and comments trivial fixes

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 15:50 +0200
SubjectRe: [PATCH 03/44] kdbus: Kernel-docs and comments trivial fixes
Message-ID<qhjK2-58q-9@gated-at.bofh.it>
In reply to#1242268
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
>  - kdbus_conn_unref(): Fix style issue to stay similar to other
>    kernel-docs.
>
>  - kdbus_conn_disconnect(): Update the name of parameter:
>    "ensure_msg_list_empty" -> "ensure_queue_empty".
>
>  - kdbus_conn_entry_insert(): Fix typo: dot -> comma.
>
>  - struct kdbus_conn: Remove lost "activator for" string.
>
>  - kdbus_fs_init(): Fix typo: "nameing" -> "naming".
>
>  - kdbus_node_deactivate(): Fix typo in comment:
>    "node as children" -> "node has children".
>
>  - kdbus_pool_slice_alloc(): Remove description of kvec and iovec as
>    they relate to the old code.
>
>  - struct kdbus_queue_entry: Fix typo: "messages" -> "message".
>
>  - kdbus_pool_slice_publish(): Remove obsolete line from the
>    description.
>
> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/connection.c | 6 +++---
>  ipc/kdbus/connection.h | 1 -
>  ipc/kdbus/fs.c         | 2 +-
>  ipc/kdbus/node.c       | 4 ++--
>  ipc/kdbus/pool.c       | 5 +----
>  ipc/kdbus/queue.h      | 2 +-
>  6 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index ef63d6533273..4f3cd370ecd9 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -317,7 +317,7 @@ struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn)
>
>  /**
>   * kdbus_conn_unref() - drop a connection reference
> - * @conn:              Connection (may be NULL)
> + * @conn:              Connection, may be %NULL

We usually use "Foobar, or NULL" in kdbus docs, and then describe the
case for NULL in the description.

Otherwise, looks good, thanks!
David

>   *
>   * When the last reference is dropped, the connection's internal structure
>   * is freed.
> @@ -490,7 +490,7 @@ exit_disconnect:
>   *                     case the connection's message list is not
>   *                     empty
>   *
> - * If @ensure_msg_list_empty is true, and the connection has pending messages,
> + * If @ensure_queue_empty is true, and the connection has pending messages,
>   * -EBUSY is returned.
>   *
>   * Return: 0 on success, negative errno on failure
> @@ -880,7 +880,7 @@ static int kdbus_conn_entry_sync_attach(struct kdbus_conn *conn_dst,
>   * @reply:             The reply tracker to attach to the queue entry
>   * @name:              Destination name this msg is sent to, or NULL
>   *
> - * Return: 0 on success. negative error otherwise.
> + * Return: 0 on success, negative error otherwise.
>   */
>  int kdbus_conn_entry_insert(struct kdbus_conn *conn_src,
>                             struct kdbus_conn *conn_dst,
> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index 1ad082014faa..679f393d3e68 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -53,7 +53,6 @@ struct kdbus_staging;
>   * @reply_list:                List of connections this connection should
>   *                     reply to
>   * @work:              Delayed work to handle timeouts
> - *                     activator for
>   * @match_db:          Subscription filter to broadcast messages
>   * @meta_proc:         Process metadata of connection creator, or NULL
>   * @meta_fake:         Faked metadata, or NULL
> diff --git a/ipc/kdbus/fs.c b/ipc/kdbus/fs.c
> index 09c480924b9e..47e652f2ee0e 100644
> --- a/ipc/kdbus/fs.c
> +++ b/ipc/kdbus/fs.c
> @@ -383,7 +383,7 @@ static struct file_system_type fs_type = {
>   * kdbus_fs_init() - register kdbus filesystem
>   *
>   * This registers a filesystem with the VFS layer. The filesystem is called
> - * `KBUILD_MODNAME "fs"', which usually resolves to `kdbusfs'. The nameing
> + * `KBUILD_MODNAME "fs"', which usually resolves to `kdbusfs'. The naming
>   * scheme allows to set KBUILD_MODNAME to "kdbus2" and you will get an
>   * independent filesystem for developers.
>   *
> diff --git a/ipc/kdbus/node.c b/ipc/kdbus/node.c
> index 89f58bc85433..133fb01a35cc 100644
> --- a/ipc/kdbus/node.c
> +++ b/ipc/kdbus/node.c
> @@ -557,8 +557,8 @@ void kdbus_node_deactivate(struct kdbus_node *node)
>         /*
>          * To avoid recursion, we perform back-tracking while deactivating
>          * nodes. For each node we enter, we first mark the active-counter as
> -        * deactivated by adding BIAS. If the node as children, we set the first
> -        * child as current position and start over. If the node has no
> +        * deactivated by adding BIAS. If the node has children, we set the
> +        * first child as current position and start over. If the node has no
>          * children, we drain the node by waiting for all active refs to be
>          * dropped and then releasing the node.
>          *
> diff --git a/ipc/kdbus/pool.c b/ipc/kdbus/pool.c
> index 63ccd55713c7..0433e26b777e 100644
> --- a/ipc/kdbus/pool.c
> +++ b/ipc/kdbus/pool.c
> @@ -192,9 +192,7 @@ static struct kdbus_pool_slice *kdbus_pool_find_slice(struct kdbus_pool *pool,
>   * @accounted: Whether this slice should be accounted for
>   *
>   * The returned slice is used for kdbus_pool_slice_release() to
> - * free the allocated memory. If either @kvec or @iovec is non-NULL, the data
> - * will be copied from kernel or userspace memory into the new slice at
> - * offset 0.
> + * free the allocated memory.
>   *
>   * Return: the allocated slice on success, ERR_PTR on failure.
>   */
> @@ -411,7 +409,6 @@ void kdbus_pool_publish_empty(struct kdbus_pool *pool, u64 *off, u64 *size)
>   * This prepares a slice to be published to user-space.
>   *
>   * This call combines the following operations:
> - *   * the memory region is flushed so the user's memory view is consistent
>   *   * the slice is marked as referenced by user-space, so user-space has to
>   *     call KDBUS_CMD_FREE to release it
>   *   * the offset and size of the slice are written to the given output
> diff --git a/ipc/kdbus/queue.h b/ipc/kdbus/queue.h
> index bf686d182ce1..92f7549d8c9b 100644
> --- a/ipc/kdbus/queue.h
> +++ b/ipc/kdbus/queue.h
> @@ -38,7 +38,7 @@ struct kdbus_queue {
>  };
>
>  /**
> - * struct kdbus_queue_entry - messages waiting to be read
> + * struct kdbus_queue_entry - message waiting to be read
>   * @entry:             Entry in the connection's list
>   * @prio_node:         Entry in the priority queue tree
>   * @prio_entry:                Queue tree node entry in the list of one priority
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242270 — [PATCH 28/44] kdbus: Cleanup kdbus_queue_entry_new()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 28/44] kdbus: Cleanup kdbus_queue_entry_new()
Message-ID<qhhRV-2oG-39@gated-at.bofh.it>
In reply to#1242233
Remove separate exit point as the only error case in the function is
when kdbus_staging_emit() fails. Assign a slice to temporary var first
to not clear it explicitly on error and to return an error code without
`ret' variable.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/queue.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
index 90e8d16f5967..3c0fb3bb55da 100644
--- a/ipc/kdbus/queue.c
+++ b/ipc/kdbus/queue.c
@@ -185,7 +185,7 @@ struct kdbus_queue_entry *kdbus_queue_entry_new(struct kdbus_conn *src,
 						struct kdbus_staging *s)
 {
 	struct kdbus_queue_entry *entry;
-	int ret;
+	struct kdbus_pool_slice *slice;
 
 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
@@ -196,19 +196,15 @@ struct kdbus_queue_entry *kdbus_queue_entry_new(struct kdbus_conn *src,
 	entry->conn = kdbus_conn_ref(dst);
 	entry->gaps = kdbus_gaps_ref(s->gaps);
 
-	entry->slice = kdbus_staging_emit(s, src, dst);
-	if (IS_ERR(entry->slice)) {
-		ret = PTR_ERR(entry->slice);
-		entry->slice = NULL;
-		goto error;
+	slice = kdbus_staging_emit(s, src, dst);
+	if (IS_ERR(slice)) {
+		kdbus_queue_entry_free(entry);
+		return ERR_CAST(slice);
 	}
 
+	entry->slice = slice;
 	entry->user = src ? kdbus_user_ref(src->user) : NULL;
 	return entry;
-
-error:
-	kdbus_queue_entry_free(entry);
-	return ERR_PTR(ret);
 }
 
 /**
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242271 — [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info()
Message-ID<qhhRU-2oG-31@gated-at.bofh.it>
In reply to#1242233
 - 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;
+
 		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;
 
 exit:
 	up_read(&bus->name_registry->rwlock);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242437 — Re: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:40 +0200
SubjectRe: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info()
Message-ID<qhkwq-6iy-17@gated-at.bofh.it>
In reply to#1242271
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243603 — Re: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-09 20:50 +0200
SubjectRe: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info()
Message-ID<qhKTT-231-11@gated-at.bofh.it>
In reply to#1242437
Hi,

On Thu, Oct 08, 2015 at 04:38:11PM +0200, David Herrmann wrote:
> 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.

This change was intended to stress to one who is reading the code that
kdbus_pool_slice_copy_kvec() returns >= 0 on success (as it returns the
number of bytes copied in contrast to most of functions which return 0 on
success). The only reason we have 'ret = 0' in the end of the function
is to reset `ret' after kdbus_pool_slice_copy_kvec(), so I decided to
group it together. Without kdbus_pool_slice_copy_kvec() we could return
`ret' as is, because it is always zero on success path.

BTW, kdbus_node_link() for example does the same: uses `ret' to handle
the return val of ida_simple_get() and then reset it to zero
immediately.

But that's too much words on such a simple change. I don't mind omitting
it from v2.

> 
> Thanks
> David
> 
> >
> >  exit:
> >         up_read(&bus->name_registry->rwlock);
> > --
> > 1.8.3.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242272 — [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 26/44] kdbus: Cleanup kdbus_pin_dst()
Message-ID<qhhRV-2oG-41@gated-at.bofh.it>
In reply to#1242233
 - 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;
 		}
 	} else {
+		struct kdbus_name_owner *owner;
+
 		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;
 		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;
 }
 
 static int kdbus_conn_reply(struct kdbus_conn *src,
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242444 — Re: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:50 +0200
SubjectRe: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst()
Message-ID<qhkG5-6u9-13@gated-at.bofh.it>
In reply to#1242272
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.

>                 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243606 — Re: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-09 20:50 +0200
SubjectRe: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst()
Message-ID<qhKTU-231-17@gated-at.bofh.it>
In reply to#1242444
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
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242273 — [PATCH 09/44] kdbus: Remove unused KDBUS_MSG_MAX_SIZE constant

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 09/44] kdbus: Remove unused KDBUS_MSG_MAX_SIZE constant
Message-ID<qhhRV-2oG-43@gated-at.bofh.it>
In reply to#1242233
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/limits.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ipc/kdbus/limits.h b/ipc/kdbus/limits.h
index bd47119cdf1b..5e4d2b5b0a9d 100644
--- a/ipc/kdbus/limits.h
+++ b/ipc/kdbus/limits.h
@@ -16,9 +16,6 @@
 
 #include <linux/kernel.h>
 
-/* maximum size of message header and items */
-#define KDBUS_MSG_MAX_SIZE		SZ_8K
-
 /* maximum number of memfd items per message */
 #define KDBUS_MSG_MAX_MEMFD_ITEMS	16
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242274 — [PATCH 19/44] kdbus: Drop useless initialization from kdbus_conn_reply()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 19/44] kdbus: Drop useless initialization from kdbus_conn_reply()
Message-ID<qhhRV-2oG-55@gated-at.bofh.it>
In reply to#1242233
`name' is assigned the value at first use by kdbus_pin_dst(). Do not
initialize it during declaration.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/connection.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 185ed3ba1bce..4b5ed4bb59c7 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -1104,7 +1104,7 @@ static int kdbus_conn_reply(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 *reply, *wake = NULL;
 	struct kdbus_conn *dst = NULL;
 	struct kdbus_bus *bus = src->ep->bus;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242276 — [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert()
Message-ID<qhhRV-2oG-47@gated-at.bofh.it>
In reply to#1242233
Assign zero to `ret' in the beginning of function instead of doing it
in the end.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/connection.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 4f3cd370ecd9..185ed3ba1bce 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -889,7 +889,7 @@ int kdbus_conn_entry_insert(struct kdbus_conn *conn_src,
 			    const struct kdbus_name_entry *name)
 {
 	struct kdbus_queue_entry *entry;
-	int ret;
+	int ret = 0;
 
 	kdbus_conn_lock2(conn_src, conn_dst);
 
@@ -916,8 +916,6 @@ int kdbus_conn_entry_insert(struct kdbus_conn *conn_src,
 	kdbus_queue_entry_enqueue(entry, reply);
 	wake_up_interruptible(&conn_dst->wait);
 
-	ret = 0;
-
 exit_unlock:
 	kdbus_conn_unlock2(conn_src, conn_dst);
 	return ret;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242409 — Re: [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:30 +0200
SubjectRe: [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert()
Message-ID<qhkmJ-67a-11@gated-at.bofh.it>
In reply to#1242276
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> Assign zero to `ret' in the beginning of function instead of doing it
> in the end.
>
> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/connection.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 4f3cd370ecd9..185ed3ba1bce 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -889,7 +889,7 @@ int kdbus_conn_entry_insert(struct kdbus_conn *conn_src,
>                             const struct kdbus_name_entry *name)
>  {
>         struct kdbus_queue_entry *entry;
> -       int ret;
> +       int ret = 0;
>
>         kdbus_conn_lock2(conn_src, conn_dst);
>
> @@ -916,8 +916,6 @@ int kdbus_conn_entry_insert(struct kdbus_conn *conn_src,
>         kdbus_queue_entry_enqueue(entry, reply);
>         wake_up_interruptible(&conn_dst->wait);
>
> -       ret = 0;
> -

Not a big fan of this. It makes it less obvious, and this style is
wrong in several cases (but not here). We often only check for "ret <
0", but generally want >0 to be turned into 0 on return.

It does not matter in this specific case, but I prefer making return
codes explicit, rather than relying on a previous initialization to be
still valid.

What's your rationale here?

Thanks
David

>  exit_unlock:
>         kdbus_conn_unlock2(conn_src, conn_dst);
>         return ret;
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243564 — Re: [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-09 20:00 +0200
SubjectRe: [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert()
Message-ID<qhK7v-Tp-13@gated-at.bofh.it>
In reply to#1242409
Hi,

On Thu, Oct 08, 2015 at 04:28:29PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> > Assign zero to `ret' in the beginning of function instead of doing it
> > in the end.
> >
> > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> > ---
> >  ipc/kdbus/connection.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> > index 4f3cd370ecd9..185ed3ba1bce 100644
> > --- a/ipc/kdbus/connection.c
> > +++ b/ipc/kdbus/connection.c
> > @@ -889,7 +889,7 @@ int kdbus_conn_entry_insert(struct kdbus_conn *conn_src,
> >                             const struct kdbus_name_entry *name)
> >  {
> >         struct kdbus_queue_entry *entry;
> > -       int ret;
> > +       int ret = 0;
> >
> >         kdbus_conn_lock2(conn_src, conn_dst);
> >
> > @@ -916,8 +916,6 @@ int kdbus_conn_entry_insert(struct kdbus_conn *conn_src,
> >         kdbus_queue_entry_enqueue(entry, reply);
> >         wake_up_interruptible(&conn_dst->wait);
> >
> > -       ret = 0;
> > -
> 
> Not a big fan of this. It makes it less obvious, and this style is
> wrong in several cases (but not here). We often only check for "ret <
> 0", but generally want >0 to be turned into 0 on return.
> 
> It does not matter in this specific case, but I prefer making return
> codes explicit, rather than relying on a previous initialization to be
> still valid.
> 
> What's your rationale here?

The rationale is to keep things simple. That `ret' var is used only once
to deliver the error code, and the function itself has only two local
vars and fits into my 12.5 inch thinkpad screen, so IMO that extra line
with assignment is redundant. I agree that in some cases we need to
handle 'ret > 0', but using same templates for every particular case
produces boring code :)

And BTW we have this style in number of places over kdbus code. For
example see kdbus_handle_ioctl_control(), kdbus_handle_ioctl_ep(),
kdbus_name_update(), kdbus_name_release(), kdbus_pool_release_offset(),
kdbus_pool_slice_copy().

> 
> Thanks
> David
> 
> >  exit_unlock:
> >         kdbus_conn_unlock2(conn_src, conn_dst);
> >         return ret;
> > --
> > 1.8.3.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242278 — [PATCH 06/44] kdbus: Fix kernel-doc for struct kdbus_gaps

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 06/44] kdbus: Fix kernel-doc for struct kdbus_gaps
Message-ID<qhhRV-2oG-53@gated-at.bofh.it>
In reply to#1242233
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/message.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ipc/kdbus/message.h b/ipc/kdbus/message.h
index 298f9c99dfcf..405a5668f9d5 100644
--- a/ipc/kdbus/message.h
+++ b/ipc/kdbus/message.h
@@ -27,11 +27,12 @@ struct kdbus_pool_slice;
 /**
  * struct kdbus_gaps - gaps in message to be filled later
  * @kref:		Reference counter
- * @n_memfd_offs:	Number of memfds
- * @memfd_offs:		Offsets of kdbus_memfd items in target slice
+ * @n_memfds:		Number of memfds
+ * @memfd_offsets:	Offsets of kdbus_memfd items in target slice
+ * @memfd_files:	Array of struct file pointers representing memfds
  * @n_fds:		Number of fds
- * @fds:		Array of sent fds
- * @fds_offset:		Offset of fd-array in target slice
+ * @fd_files:		Array of struct file pointers representing fds
+ * @fd_offset:		Offset of fd-array in target slice
  *
  * The 'gaps' object is used to track data that is needed to fill gaps in a
  * message at RECV time. Usually, we try to compile the whole message at SEND
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242529

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 17:30 +0200
Message-ID<qhliO-7tw-27@gated-at.bofh.it>
In reply to#1242233
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> Hi all,
>
> This is a set of various kdbus code cleanups. Patches are ordered by
> increasing complexity, starting with docs and comments fixes and
> one-liners.
>
> Patch 29 is the revised version of
> http://lkml.kernel.org/g/1435497454-10464-6-git-send-email-sergei@s15v.net
>
> Feel free to ask to change layout of this, split/join, etc if necessary.

So I reviewed all of the patches, most of them look good. Some comments:
 - Please justify your changes in the commit-message. Always.
 - Please don't split patches based on modified functions. If you fix
typos, do them subsystem-wide. If you fix common errors like "don't
init 'name' to NULL before calling kdbus_pin_dst()", then please do
that for all functions. In other words, group your changes logically,
not based on location.
 - If you do cleanups, explain why you do them. I commented on some of
the changes, which IMO reduce readability.

Anyway, looks good.

Thanks a lot!
David

> Thanks, Sergei
>
> Sergei Zviagintsev (44):
>   Documentation/kdbus: Document new name registry flags
>   uapi: kdbus.h: Kernel-doc fixes
>   kdbus: Kernel-docs and comments trivial fixes
>   kdbus: Update kernel-doc for struct kdbus_pool
>   kdbus: Add comment on merging free pool slices
>   kdbus: Fix kernel-doc for struct kdbus_gaps
>   kdbus: Fix comment on translation of caps between namespaces
>   kdbus: Rename var in kdbus_meta_export_caps()
>   kdbus: Remove unused KDBUS_MSG_MAX_SIZE constant
>   kdbus: Use conditional operator
>   kdbus: Cosmetic fix of kdbus_name_is_valid()
>   kdbus: Use conventional list macros in __kdbus_pool_slice_release()
>   kdbus: Use list_next_entry() in kdbus_queue_entry_unlink()
>   kdbus: Simplify expression in kdbus_get_memfd()
>   kdbus: Simplify bitwise expression in kdbus_meta_get_mask()
>   kdbus: Drop redundant code from kdbus_name_acquire()
>   kdbus: Drop duplicated code from kdbus_pool_slice_alloc()
>   kdbus: Add var initialization to kdbus_conn_entry_insert()
>   kdbus: Drop useless initialization from kdbus_conn_reply()
>   kdbus: Drop useless initialization from kdbus_cmd_hello()
>   kdbus: Cleanup tests in kdbus_cmd_send()
>   kdbus: Cleanup error path in kdbus_staging_new_user()
>   kdbus: Cleanup kdbus_conn_call()
>   kdbus: Cleanup kdbus_conn_unicast()
>   kdbus: Cleanup kdbus_cmd_conn_info()
>   kdbus: Cleanup kdbus_pin_dst()
>   kdbus: Cleanup kdbus_conn_new()
>   kdbus: Cleanup kdbus_queue_entry_new()
>   kdbus: Improve tests on incrementing quota
>   kdbus: Cleanup kdbus_meta_proc_mask()
>   kdbus: Cleanup kdbus_conn_move_messages()
>   kdbus: Remove duplicated code from kdbus_conn_lock2()
>   kdbus: Improve kdbus_staging_reserve()
>   kdbus: Improve kdbus_conn_entry_sync_attach()
>   kdbus: Drop goto from kdbus_queue_entry_link()
>   kdbus: Improve kdbus_name_release()
>   kdbus: Fix error path in kdbus_meta_proc_collect_cgroup()
>   kdbus: Fix error path in kdbus_user_lookup()
>   kdbus: Cleanup kdbus_user_lookup()
>   kdbus: Cleanup kdbus_item_validate_name()
>   kdbus: Fix memfd install algorithm
>   kdbus: Check if fd is allocated before trying to free it
>   kdbus: Give up on failed fd allocation
>   kdbus: Cleanup kdbus_gaps_install()
>
>  Documentation/kdbus/kdbus.name.xml |  42 +++++++++-
>  include/uapi/linux/kdbus.h         |  43 +++++-----
>  ipc/kdbus/connection.c             | 157 +++++++++++++++----------------------
>  ipc/kdbus/connection.h             |  19 ++---
>  ipc/kdbus/domain.c                 |  38 +++++----
>  ipc/kdbus/fs.c                     |   2 +-
>  ipc/kdbus/item.c                   |  26 +++---
>  ipc/kdbus/limits.h                 |   3 -
>  ipc/kdbus/message.c                |  81 +++++++++----------
>  ipc/kdbus/message.h                |   9 ++-
>  ipc/kdbus/metadata.c               |  79 ++++++++++---------
>  ipc/kdbus/names.c                  |  32 ++++----
>  ipc/kdbus/node.c                   |   4 +-
>  ipc/kdbus/pool.c                   |  26 +++---
>  ipc/kdbus/queue.c                  |  51 ++++++------
>  ipc/kdbus/queue.h                  |   2 +-
>  16 files changed, 298 insertions(+), 316 deletions(-)
>
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 3 of 4 — ← Prev page 1 2 [3] 4  Next page →

Back to top | Article view | linux.kernel

csiph-web