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 2 of 4 — ← Prev page 1 [2] 3 4  Next page →

#1242251 — [PATCH 22/44] kdbus: Cleanup error path in kdbus_staging_new_user()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 22/44] kdbus: Cleanup error path in kdbus_staging_new_user()
Message-ID<qhhIg-2d4-65@gated-at.bofh.it>
In reply to#1242233
Replace duplicated cleanup code with jump to exit point.

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

diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index c7ef23d40471..e337b1b1024a 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -800,8 +800,7 @@ struct kdbus_staging *kdbus_staging_new_user(struct kdbus_bus *bus,
 		if (IS_ERR(staging->gaps)) {
 			ret = PTR_ERR(staging->gaps);
 			staging->gaps = NULL;
-			kdbus_staging_free(staging);
-			return ERR_PTR(ret);
+			goto error;
 		}
 	}
 
-- 
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]

#1242253 — [PATCH 05/44] kdbus: Add comment on merging free pool slices

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 05/44] kdbus: Add comment on merging free pool slices
Message-ID<qhhIg-2d4-67@gated-at.bofh.it>
In reply to#1242233
Add comment on why we remove the same slice from free slices tree and
then add it back again when merging the slice to be released with
previous free slice.

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

diff --git a/ipc/kdbus/pool.c b/ipc/kdbus/pool.c
index 84afe96fbc22..c26ef963d8d1 100644
--- a/ipc/kdbus/pool.c
+++ b/ipc/kdbus/pool.c
@@ -304,6 +304,12 @@ static void __kdbus_pool_slice_release(struct kdbus_pool_slice *slice)
 		s = list_entry(slice->entry.prev,
 			       struct kdbus_pool_slice, entry);
 		if (s->free) {
+			/*
+			 * As size of slice increases after merge and free
+			 * slices tree is ordered by slice size, we have to
+			 * remove the slice from free slices tree and then add
+			 * it again to keep the tree balanced.
+			 */
 			rb_erase(&s->rb_node, &pool->slices_free);
 			list_del(&slice->entry);
 			s->size += slice->size;
-- 
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]

#1242382 — Re: [PATCH 05/44] kdbus: Add comment on merging free pool slices

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:00 +0200
SubjectRe: [PATCH 05/44] kdbus: Add comment on merging free pool slices
Message-ID<qhjTI-5jJ-15@gated-at.bofh.it>
In reply to#1242253
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> Add comment on why we remove the same slice from free slices tree and
> then add it back again when merging the slice to be released with
> previous free slice.
>
> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/pool.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/ipc/kdbus/pool.c b/ipc/kdbus/pool.c
> index 84afe96fbc22..c26ef963d8d1 100644
> --- a/ipc/kdbus/pool.c
> +++ b/ipc/kdbus/pool.c
> @@ -304,6 +304,12 @@ static void __kdbus_pool_slice_release(struct kdbus_pool_slice *slice)
>                 s = list_entry(slice->entry.prev,
>                                struct kdbus_pool_slice, entry);
>                 if (s->free) {
> +                       /*
> +                        * As size of slice increases after merge and free
> +                        * slices tree is ordered by slice size, we have to
> +                        * remove the slice from free slices tree and then add
> +                        * it again to keep the tree balanced.
> +                        */

Mhh, isn't this obvious? "slices_free" is ordered by s->size, so a
change of the key requires a re-insert. If you disagree, maybe keep it
simple:

        /* s->size changed, re-insert slice in rbtree */

Thanks
David

>                         rb_erase(&s->rb_node, &pool->slices_free);
>                         list_del(&slice->entry);
>                         s->size += slice->size;
> --
> 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]

#1243582 — Re: [PATCH 05/44] kdbus: Add comment on merging free pool slices

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-09 20:20 +0200
SubjectRe: [PATCH 05/44] kdbus: Add comment on merging free pool slices
Message-ID<qhKqS-1vd-25@gated-at.bofh.it>
In reply to#1242382
Hi,

On Thu, Oct 08, 2015 at 03:50:46PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> > Add comment on why we remove the same slice from free slices tree and
> > then add it back again when merging the slice to be released with
> > previous free slice.
> >
> > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> > ---
> >  ipc/kdbus/pool.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/ipc/kdbus/pool.c b/ipc/kdbus/pool.c
> > index 84afe96fbc22..c26ef963d8d1 100644
> > --- a/ipc/kdbus/pool.c
> > +++ b/ipc/kdbus/pool.c
> > @@ -304,6 +304,12 @@ static void __kdbus_pool_slice_release(struct kdbus_pool_slice *slice)
> >                 s = list_entry(slice->entry.prev,
> >                                struct kdbus_pool_slice, entry);
> >                 if (s->free) {
> > +                       /*
> > +                        * As size of slice increases after merge and free
> > +                        * slices tree is ordered by slice size, we have to
> > +                        * remove the slice from free slices tree and then add
> > +                        * it again to keep the tree balanced.
> > +                        */
> 
> Mhh, isn't this obvious? "slices_free" is ordered by s->size, so a
> change of the key requires a re-insert. If you disagree, maybe keep it
> simple:
> 
>         /* s->size changed, re-insert slice in rbtree */

It wasn't obvious for me from the first sight (but don't tell anyone),
so I decided that some words on why we're doing re-insert would be
helpful. It's up to you to decide whether it would be useful for the
others. BTW, I find your shorter variant better.

> 
> Thanks
> David
> 
> >                         rb_erase(&s->rb_node, &pool->slices_free);
> >                         list_del(&slice->entry);
> >                         s->size += slice->size;
> > --
> > 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]

#1242254 — [PATCH 02/44] uapi: kdbus.h: Kernel-doc fixes

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 02/44] uapi: kdbus.h: Kernel-doc fixes
Message-ID<qhhIg-2d4-71@gated-at.bofh.it>
In reply to#1242233
 - Fix typos and spelling errors, use proper case.

 - struct kdbus_pids: Add PPID to description.

 - struct kdbus_item: Add missing @pids, @fsd and
   KDBUS_ITEM_PAYLOAD_OFF.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 include/uapi/linux/kdbus.h | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/include/uapi/linux/kdbus.h b/include/uapi/linux/kdbus.h
index 4fc44cb1d4a8..09b23af87ce6 100644
--- a/include/uapi/linux/kdbus.h
+++ b/include/uapi/linux/kdbus.h
@@ -21,7 +21,7 @@
 /**
  * struct kdbus_notify_id_change - name registry change message
  * @id:			New or former owner of the name
- * @flags:		flags field from KDBUS_HELLO_*
+ * @flags:		Flags field from KDBUS_HELLO_*
  *
  * Sent from kernel to userspace when the owner or activator of
  * a well-known name changes.
@@ -86,7 +86,7 @@ struct kdbus_creds {
  * @tid:		Thread ID
  * @ppid:		Parent process ID
  *
- * The PID and TID of a process.
+ * The PID, TID and PPID of a process.
  *
  * Attached to:
  *   KDBUS_ITEM_PIDS
@@ -212,7 +212,7 @@ struct kdbus_name {
  * enum kdbus_policy_access_type - permissions of a policy record
  * @_KDBUS_POLICY_ACCESS_NULL:	Uninitialized/invalid
  * @KDBUS_POLICY_ACCESS_USER:	Grant access to a uid
- * @KDBUS_POLICY_ACCESS_GROUP:	Grant access to gid
+ * @KDBUS_POLICY_ACCESS_GROUP:	Grant access to a gid
  * @KDBUS_POLICY_ACCESS_WORLD:	World-accessible
  */
 enum kdbus_policy_access_type {
@@ -267,7 +267,7 @@ struct kdbus_policy_access {
  * @KDBUS_ATTACH_CONN_DESCRIPTION:	The human-readable connection name
  * @_KDBUS_ATTACH_ALL:			All of the above
  * @_KDBUS_ATTACH_ANY:			Wildcard match to enable any kind of
- *					metatdata.
+ *					metadata
  */
 enum kdbus_attach_flags {
 	KDBUS_ATTACH_TIMESTAMP		=  1ULL <<  0,
@@ -308,7 +308,7 @@ enum kdbus_attach_flags {
  *					connection, carries a struct
  *					kdbus_bloom_filter
  * @KDBUS_ITEM_BLOOM_MASK:		Bloom mask used to match against a
- *					message'sbloom filter
+ *					message's bloom filter
  * @KDBUS_ITEM_DST_NAME:		Destination's well-known name
  * @KDBUS_ITEM_MAKE_NAME:		Name of domain, bus, endpoint
  * @KDBUS_ITEM_ATTACH_FLAGS_SEND:	Attach-flags, used for updating which
@@ -409,20 +409,23 @@ enum kdbus_item_type {
 /**
  * struct kdbus_item - chain of data blocks
  * @size:		Overall data record size
- * @type:		Kdbus_item type of data
+ * @type:		kdbus_item type of data
  * @data:		Generic bytes
- * @data32:		Generic 32 bit array
- * @data64:		Generic 64 bit array
+ * @data32:		Generic 32-bit array
+ * @data64:		Generic 64-bit array
  * @str:		Generic string
  * @id:			Connection ID
  * @vec:		KDBUS_ITEM_PAYLOAD_VEC
+ *			KDBUS_ITEM_PAYLOAD_OFF
  * @creds:		KDBUS_ITEM_CREDS
+ * @pids:		KDBUS_ITEM_PIDS
  * @audit:		KDBUS_ITEM_AUDIT
  * @timestamp:		KDBUS_ITEM_TIMESTAMP
  * @name:		KDBUS_ITEM_NAME
  * @bloom_parameter:	KDBUS_ITEM_BLOOM_PARAMETER
  * @bloom_filter:	KDBUS_ITEM_BLOOM_FILTER
  * @memfd:		KDBUS_ITEM_PAYLOAD_MEMFD
+ * @fsd:		KDBUS_ITEM_FDS
  * @name_change:	KDBUS_ITEM_NAME_ADD
  *			KDBUS_ITEM_NAME_REMOVE
  *			KDBUS_ITEM_NAME_CHANGE
@@ -721,10 +724,10 @@ struct kdbus_cmd_hello {
 
 /**
  * struct kdbus_info - connection information
- * @size:		total size of the struct
- * @id:			64bit object ID
- * @flags:		object creation flags
- * @items:		list of items
+ * @size:		Total size of the struct
+ * @id:			64-bit object ID
+ * @flags:		Object creation flags
+ * @items:		List of items
  *
  * Note that the user is responsible for freeing the allocated memory with
  * the KDBUS_CMD_FREE ioctl.
@@ -738,10 +741,10 @@ struct kdbus_info {
 
 /**
  * enum kdbus_list_flags - what to include into the returned list
- * @KDBUS_LIST_UNIQUE:		active connections
- * @KDBUS_LIST_ACTIVATORS:	activator connections
- * @KDBUS_LIST_NAMES:		known well-known names
- * @KDBUS_LIST_QUEUED:		queued-up names
+ * @KDBUS_LIST_UNIQUE:		Active connections
+ * @KDBUS_LIST_ACTIVATORS:	Activator connections
+ * @KDBUS_LIST_NAMES:		Known well-known names
+ * @KDBUS_LIST_QUEUED:		Queued-up names
  */
 enum kdbus_list_flags {
 	KDBUS_LIST_UNIQUE		= 1ULL <<  0,
@@ -752,14 +755,14 @@ enum kdbus_list_flags {
 
 /**
  * struct kdbus_cmd_list - list connections
- * @size:		overall size of this object
- * @flags:		flags for the query (KDBUS_LIST_*), userspace → kernel
- * @return_flags:	command return flags, kernel → userspace
+ * @size:		Overall size of this object
+ * @flags:		Flags for the query (KDBUS_LIST_*), userspace → kernel
+ * @return_flags:	Command return flags, kernel → userspace
  * @offset:		Offset in the caller's pool buffer where an array of
  *			kdbus_info objects is stored.
  *			The user must use KDBUS_CMD_FREE to free the
  *			allocated memory.
- * @list_size:		size of returned list in bytes
+ * @list_size:		Size of returned list in bytes
  * @items:		Items for the command. Reserved for future use.
  *
  * This structure is used with the KDBUS_CMD_LIST ioctl.
-- 
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]

#1242377 — Re: [PATCH 02/44] uapi: kdbus.h: Kernel-doc fixes

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 15:50 +0200
SubjectRe: [PATCH 02/44] uapi: kdbus.h: Kernel-doc fixes
Message-ID<qhjK3-58q-35@gated-at.bofh.it>
In reply to#1242254
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
>  - Fix typos and spelling errors, use proper case.
>
>  - struct kdbus_pids: Add PPID to description.
>
>  - struct kdbus_item: Add missing @pids, @fsd and
>    KDBUS_ITEM_PAYLOAD_OFF.
>
> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  include/uapi/linux/kdbus.h | 43 +++++++++++++++++++++++--------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/include/uapi/linux/kdbus.h b/include/uapi/linux/kdbus.h
> index 4fc44cb1d4a8..09b23af87ce6 100644
> --- a/include/uapi/linux/kdbus.h
> +++ b/include/uapi/linux/kdbus.h
> @@ -21,7 +21,7 @@
>  /**
>   * struct kdbus_notify_id_change - name registry change message
>   * @id:                        New or former owner of the name
> - * @flags:             flags field from KDBUS_HELLO_*
> + * @flags:             Flags field from KDBUS_HELLO_*
>   *
>   * Sent from kernel to userspace when the owner or activator of
>   * a well-known name changes.
> @@ -86,7 +86,7 @@ struct kdbus_creds {
>   * @tid:               Thread ID
>   * @ppid:              Parent process ID
>   *
> - * The PID and TID of a process.
> + * The PID, TID and PPID of a process.
>   *
>   * Attached to:
>   *   KDBUS_ITEM_PIDS
> @@ -212,7 +212,7 @@ struct kdbus_name {
>   * enum kdbus_policy_access_type - permissions of a policy record
>   * @_KDBUS_POLICY_ACCESS_NULL: Uninitialized/invalid
>   * @KDBUS_POLICY_ACCESS_USER:  Grant access to a uid
> - * @KDBUS_POLICY_ACCESS_GROUP: Grant access to gid
> + * @KDBUS_POLICY_ACCESS_GROUP: Grant access to a gid
>   * @KDBUS_POLICY_ACCESS_WORLD: World-accessible
>   */
>  enum kdbus_policy_access_type {
> @@ -267,7 +267,7 @@ struct kdbus_policy_access {
>   * @KDBUS_ATTACH_CONN_DESCRIPTION:     The human-readable connection name
>   * @_KDBUS_ATTACH_ALL:                 All of the above
>   * @_KDBUS_ATTACH_ANY:                 Wildcard match to enable any kind of
> - *                                     metatdata.
> + *                                     metadata
>   */
>  enum kdbus_attach_flags {
>         KDBUS_ATTACH_TIMESTAMP          =  1ULL <<  0,
> @@ -308,7 +308,7 @@ enum kdbus_attach_flags {
>   *                                     connection, carries a struct
>   *                                     kdbus_bloom_filter
>   * @KDBUS_ITEM_BLOOM_MASK:             Bloom mask used to match against a
> - *                                     message'sbloom filter
> + *                                     message's bloom filter

This actually ought to be "against the bloom filter of a message".

>   * @KDBUS_ITEM_DST_NAME:               Destination's well-known name
>   * @KDBUS_ITEM_MAKE_NAME:              Name of domain, bus, endpoint
>   * @KDBUS_ITEM_ATTACH_FLAGS_SEND:      Attach-flags, used for updating which
> @@ -409,20 +409,23 @@ enum kdbus_item_type {
>  /**
>   * struct kdbus_item - chain of data blocks
>   * @size:              Overall data record size
> - * @type:              Kdbus_item type of data
> + * @type:              kdbus_item type of data

What's "kdbus_item" doing here, anyway? I'd just drop it entirely and
make it say "Type of data".

>   * @data:              Generic bytes
> - * @data32:            Generic 32 bit array
> - * @data64:            Generic 64 bit array
> + * @data32:            Generic 32-bit array
> + * @data64:            Generic 64-bit array
>   * @str:               Generic string
>   * @id:                        Connection ID
>   * @vec:               KDBUS_ITEM_PAYLOAD_VEC
> + *                     KDBUS_ITEM_PAYLOAD_OFF
>   * @creds:             KDBUS_ITEM_CREDS
> + * @pids:              KDBUS_ITEM_PIDS
>   * @audit:             KDBUS_ITEM_AUDIT
>   * @timestamp:         KDBUS_ITEM_TIMESTAMP
>   * @name:              KDBUS_ITEM_NAME
>   * @bloom_parameter:   KDBUS_ITEM_BLOOM_PARAMETER
>   * @bloom_filter:      KDBUS_ITEM_BLOOM_FILTER
>   * @memfd:             KDBUS_ITEM_PAYLOAD_MEMFD
> + * @fsd:               KDBUS_ITEM_FDS

Typo: @fds

Thanks
David

>   * @name_change:       KDBUS_ITEM_NAME_ADD
>   *                     KDBUS_ITEM_NAME_REMOVE
>   *                     KDBUS_ITEM_NAME_CHANGE
> @@ -721,10 +724,10 @@ struct kdbus_cmd_hello {
>
>  /**
>   * struct kdbus_info - connection information
> - * @size:              total size of the struct
> - * @id:                        64bit object ID
> - * @flags:             object creation flags
> - * @items:             list of items
> + * @size:              Total size of the struct
> + * @id:                        64-bit object ID
> + * @flags:             Object creation flags
> + * @items:             List of items
>   *
>   * Note that the user is responsible for freeing the allocated memory with
>   * the KDBUS_CMD_FREE ioctl.
> @@ -738,10 +741,10 @@ struct kdbus_info {
>
>  /**
>   * enum kdbus_list_flags - what to include into the returned list
> - * @KDBUS_LIST_UNIQUE:         active connections
> - * @KDBUS_LIST_ACTIVATORS:     activator connections
> - * @KDBUS_LIST_NAMES:          known well-known names
> - * @KDBUS_LIST_QUEUED:         queued-up names
> + * @KDBUS_LIST_UNIQUE:         Active connections
> + * @KDBUS_LIST_ACTIVATORS:     Activator connections
> + * @KDBUS_LIST_NAMES:          Known well-known names
> + * @KDBUS_LIST_QUEUED:         Queued-up names
>   */
>  enum kdbus_list_flags {
>         KDBUS_LIST_UNIQUE               = 1ULL <<  0,
> @@ -752,14 +755,14 @@ enum kdbus_list_flags {
>
>  /**
>   * struct kdbus_cmd_list - list connections
> - * @size:              overall size of this object
> - * @flags:             flags for the query (KDBUS_LIST_*), userspace → kernel
> - * @return_flags:      command return flags, kernel → userspace
> + * @size:              Overall size of this object
> + * @flags:             Flags for the query (KDBUS_LIST_*), userspace → kernel
> + * @return_flags:      Command return flags, kernel → userspace
>   * @offset:            Offset in the caller's pool buffer where an array of
>   *                     kdbus_info objects is stored.
>   *                     The user must use KDBUS_CMD_FREE to free the
>   *                     allocated memory.
> - * @list_size:         size of returned list in bytes
> + * @list_size:         Size of returned list in bytes
>   * @items:             Items for the command. Reserved for future use.
>   *
>   * This structure is used with the KDBUS_CMD_LIST ioctl.
> --
> 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]

#1242255 — [PATCH 30/44] kdbus: Cleanup kdbus_meta_proc_mask()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 30/44] kdbus: Cleanup kdbus_meta_proc_mask()
Message-ID<qhhIg-2d4-73@gated-at.bofh.it>
In reply to#1242233
 - Simplify if-statements

   'proc < KDBUS_META_PROC_NORMAL' test only makes sense when we call
   kdbus_proc_permission(). Include it into 'prv_pid != req_pid' branch,
   remove redundant assignment of `proc' var and reduce its scope.

 - Drop redundant binary operation

   In 'proc < KDBUS_META_PROC_NORMAL' case we firstly do
   'wanted &= ~flags' and then 'wanted & flags' in the return statement,
   which is the same as 'wanted & 0'. Return 0 instead.

 - Cosmetic cleanup of the return statement.

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

diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
index 61215a078359..b8d094d9fb56 100644
--- a/ipc/kdbus/metadata.c
+++ b/ipc/kdbus/metadata.c
@@ -1224,7 +1224,6 @@ static u64 kdbus_meta_proc_mask(struct pid *prv_pid,
 				u64 wanted)
 {
 	struct pid_namespace *prv_ns, *req_ns;
-	unsigned int proc;
 
 	prv_ns = ns_of_pid(prv_pid);
 	req_ns = ns_of_pid(req_pid);
@@ -1243,30 +1242,23 @@ static u64 kdbus_meta_proc_mask(struct pid *prv_pid,
 	 * provider and requestor are the same. If not, we perform rather
 	 * expensive /proc permission checks.
 	 */
-	if (prv_pid == req_pid)
-		proc = KDBUS_META_PROC_NORMAL;
-	else
-		proc = kdbus_proc_permission(req_ns, req_cred, prv_pid);
-
-	/* you need /proc access to read standard process attributes */
-	if (proc < KDBUS_META_PROC_NORMAL)
-		wanted &= ~(KDBUS_ATTACH_TID_COMM |
-			    KDBUS_ATTACH_PID_COMM |
-			    KDBUS_ATTACH_SECLABEL |
-			    KDBUS_ATTACH_CMDLINE |
-			    KDBUS_ATTACH_CGROUP |
-			    KDBUS_ATTACH_AUDIT |
-			    KDBUS_ATTACH_CAPS |
-			    KDBUS_ATTACH_EXE);
+	if (prv_pid != req_pid) {
+		unsigned int proc = kdbus_proc_permission(req_ns, req_cred,
+							  prv_pid);
+
+		/* you need /proc access to read standard process attributes */
+		if (proc < KDBUS_META_PROC_NORMAL)
+			return 0;
+	}
 
 	/* clear all non-/proc flags */
 	return wanted & (KDBUS_ATTACH_TID_COMM |
 			 KDBUS_ATTACH_PID_COMM |
 			 KDBUS_ATTACH_SECLABEL |
-			 KDBUS_ATTACH_CMDLINE |
-			 KDBUS_ATTACH_CGROUP |
-			 KDBUS_ATTACH_AUDIT |
-			 KDBUS_ATTACH_CAPS |
+			 KDBUS_ATTACH_CMDLINE  |
+			 KDBUS_ATTACH_CGROUP   |
+			 KDBUS_ATTACH_AUDIT    |
+			 KDBUS_ATTACH_CAPS     |
 			 KDBUS_ATTACH_EXE);
 }
 
-- 
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]

#1242459 — Re: [PATCH 30/44] kdbus: Cleanup kdbus_meta_proc_mask()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:50 +0200
SubjectRe: [PATCH 30/44] kdbus: Cleanup kdbus_meta_proc_mask()
Message-ID<qhkG7-6u9-53@gated-at.bofh.it>
In reply to#1242255
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
>  - Simplify if-statements
>
>    'proc < KDBUS_META_PROC_NORMAL' test only makes sense when we call
>    kdbus_proc_permission(). Include it into 'prv_pid != req_pid' branch,
>    remove redundant assignment of `proc' var and reduce its scope.
>
>  - Drop redundant binary operation
>
>    In 'proc < KDBUS_META_PROC_NORMAL' case we firstly do
>    'wanted &= ~flags' and then 'wanted & flags' in the return statement,
>    which is the same as 'wanted & 0'. Return 0 instead.
>
>  - Cosmetic cleanup of the return statement.
>
> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/metadata.c | 32 ++++++++++++--------------------
>  1 file changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
> index 61215a078359..b8d094d9fb56 100644
> --- a/ipc/kdbus/metadata.c
> +++ b/ipc/kdbus/metadata.c
> @@ -1224,7 +1224,6 @@ static u64 kdbus_meta_proc_mask(struct pid *prv_pid,
>                                 u64 wanted)
>  {
>         struct pid_namespace *prv_ns, *req_ns;
> -       unsigned int proc;
>
>         prv_ns = ns_of_pid(prv_pid);
>         req_ns = ns_of_pid(req_pid);
> @@ -1243,30 +1242,23 @@ static u64 kdbus_meta_proc_mask(struct pid *prv_pid,
>          * provider and requestor are the same. If not, we perform rather
>          * expensive /proc permission checks.
>          */
> -       if (prv_pid == req_pid)
> -               proc = KDBUS_META_PROC_NORMAL;
> -       else
> -               proc = kdbus_proc_permission(req_ns, req_cred, prv_pid);
> -
> -       /* you need /proc access to read standard process attributes */
> -       if (proc < KDBUS_META_PROC_NORMAL)
> -               wanted &= ~(KDBUS_ATTACH_TID_COMM |
> -                           KDBUS_ATTACH_PID_COMM |
> -                           KDBUS_ATTACH_SECLABEL |
> -                           KDBUS_ATTACH_CMDLINE |
> -                           KDBUS_ATTACH_CGROUP |
> -                           KDBUS_ATTACH_AUDIT |
> -                           KDBUS_ATTACH_CAPS |
> -                           KDBUS_ATTACH_EXE);
> +       if (prv_pid != req_pid) {
> +               unsigned int proc = kdbus_proc_permission(req_ns, req_cred,
> +                                                         prv_pid);

Please keep "proc" declared non-local. Also, we don't do direct
assignments of non-static data in declarations.

Thanks
David

> +
> +               /* you need /proc access to read standard process attributes */
> +               if (proc < KDBUS_META_PROC_NORMAL)
> +                       return 0;
> +       }
>
>         /* clear all non-/proc flags */
>         return wanted & (KDBUS_ATTACH_TID_COMM |
>                          KDBUS_ATTACH_PID_COMM |
>                          KDBUS_ATTACH_SECLABEL |
> -                        KDBUS_ATTACH_CMDLINE |
> -                        KDBUS_ATTACH_CGROUP |
> -                        KDBUS_ATTACH_AUDIT |
> -                        KDBUS_ATTACH_CAPS |
> +                        KDBUS_ATTACH_CMDLINE  |
> +                        KDBUS_ATTACH_CGROUP   |
> +                        KDBUS_ATTACH_AUDIT    |
> +                        KDBUS_ATTACH_CAPS     |
>                          KDBUS_ATTACH_EXE);
>  }
>
> --
> 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]

#1242256 — [PATCH 14/44] kdbus: Simplify expression in kdbus_get_memfd()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 14/44] kdbus: Simplify expression in kdbus_get_memfd()
Message-ID<qhhIg-2d4-79@gated-at.bofh.it>
In reply to#1242233
'(s & m) != m' means that mask 'm' contains some bits which are not set
in 's', and this is literally equal to '~s & m'.

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

diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index ae565cd343f8..c7ef23d40471 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -273,7 +273,7 @@ static struct file *kdbus_get_memfd(const struct kdbus_memfd *memfd)
 	s = shmem_get_seals(f);
 	if (s < 0)
 		ret = ERR_PTR(-EMEDIUMTYPE);
-	else if ((s & m) != m)
+	else if (~s & m)
 		ret = ERR_PTR(-ETXTBSY);
 	else if (memfd->start + memfd->size > (u64)i_size_read(file_inode(f)))
 		ret = ERR_PTR(-EFAULT);
-- 
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]

#1242412 — Re: [PATCH 14/44] kdbus: Simplify expression in kdbus_get_memfd()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:30 +0200
SubjectRe: [PATCH 14/44] kdbus: Simplify expression in kdbus_get_memfd()
Message-ID<qhkmK-67a-23@gated-at.bofh.it>
In reply to#1242256
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> '(s & m) != m' means that mask 'm' contains some bits which are not set
> in 's', and this is literally equal to '~s & m'.

Sure, but you make the code look much less obvious. Checking a bit is
set is "a & b", checking if not set is "!(a & b)". If you check
whether a whole mask is set, you run "(a & m) == m", checking whether
it not set should be the negation, which is "(a & m) != m".

I'd prefer keeping the current code, unless the compiler cannot figure
it out on its own.
David

> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/message.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
> index ae565cd343f8..c7ef23d40471 100644
> --- a/ipc/kdbus/message.c
> +++ b/ipc/kdbus/message.c
> @@ -273,7 +273,7 @@ static struct file *kdbus_get_memfd(const struct kdbus_memfd *memfd)
>         s = shmem_get_seals(f);
>         if (s < 0)
>                 ret = ERR_PTR(-EMEDIUMTYPE);
> -       else if ((s & m) != m)
> +       else if (~s & m)
>                 ret = ERR_PTR(-ETXTBSY);
>         else if (memfd->start + memfd->size > (u64)i_size_read(file_inode(f)))
>                 ret = ERR_PTR(-EFAULT);
> --
> 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]

#1242257 — [PATCH 13/44] kdbus: Use list_next_entry() in kdbus_queue_entry_unlink()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 13/44] kdbus: Use list_next_entry() in kdbus_queue_entry_unlink()
Message-ID<qhhIg-2d4-81@gated-at.bofh.it>
In reply to#1242233
Use list_next_entry() instead of list_first_entry().

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

diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
index f9c44d7bae6d..90e8d16f5967 100644
--- a/ipc/kdbus/queue.c
+++ b/ipc/kdbus/queue.c
@@ -157,8 +157,7 @@ static void kdbus_queue_entry_unlink(struct kdbus_queue_entry *entry)
 		 * the list. Update cached highest-priority entry, store the
 		 * new one as the tree node.
 		 */
-		q = list_first_entry(&entry->prio_entry,
-				     struct kdbus_queue_entry, prio_entry);
+		q = list_next_entry(entry, prio_entry);
 		list_del(&entry->prio_entry);
 
 		if (queue->msg_prio_highest == &entry->prio_node)
-- 
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]

#1242393 — Re: [PATCH 13/44] kdbus: Use list_next_entry() in kdbus_queue_entry_unlink()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:10 +0200
SubjectRe: [PATCH 13/44] kdbus: Use list_next_entry() in kdbus_queue_entry_unlink()
Message-ID<qhk3o-5Ki-15@gated-at.bofh.it>
In reply to#1242257
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> Use list_next_entry() instead of list_first_entry().

This commit-message is completely useless. This is not a cosmetic
change, so please justify your changes. Sure, in this case both
list_next_entry() and list_first_entry() perform the same operation,
but you should really describe such changes in the commit message to
allow reviewers to understand the intent of your change.

Anyway, patch looks good, thanks!
David

> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/queue.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
> index f9c44d7bae6d..90e8d16f5967 100644
> --- a/ipc/kdbus/queue.c
> +++ b/ipc/kdbus/queue.c
> @@ -157,8 +157,7 @@ static void kdbus_queue_entry_unlink(struct kdbus_queue_entry *entry)
>                  * the list. Update cached highest-priority entry, store the
>                  * new one as the tree node.
>                  */
> -               q = list_first_entry(&entry->prio_entry,
> -                                    struct kdbus_queue_entry, prio_entry);
> +               q = list_next_entry(entry, prio_entry);
>                 list_del(&entry->prio_entry);
>
>                 if (queue->msg_prio_highest == &entry->prio_node)
> --
> 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]

#1242258 — [PATCH 33/44] kdbus: Improve kdbus_staging_reserve()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 33/44] kdbus: Improve kdbus_staging_reserve()
Message-ID<qhhIg-2d4-77@gated-at.bofh.it>
In reply to#1242233
Allow to reserve N elements in row. This eliminates duplicated code.

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

diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index e337b1b1024a..f2176796390d 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -647,13 +647,14 @@ static int kdbus_staging_import(struct kdbus_staging *staging)
 	return 0;
 }
 
-static void kdbus_staging_reserve(struct kdbus_staging *staging)
+static void kdbus_staging_reserve(struct kdbus_staging *staging, size_t n)
 {
-	struct iovec *part;
+	while (n--) {
+		struct iovec *part = &staging->parts[staging->n_parts++];
 
-	part = &staging->parts[staging->n_parts++];
-	part->iov_base = (void __user *)zeros;
-	part->iov_len = 0;
+		part->iov_base = (void __user *)zeros;
+		part->iov_len = 0;
+	}
 }
 
 static struct kdbus_staging *kdbus_staging_new(struct kdbus_bus *bus,
@@ -701,16 +702,9 @@ static struct kdbus_staging *kdbus_staging_new(struct kdbus_bus *bus,
 	 *   * iovec for possible padding after the items
 	 *   * iovec for metadata items
 	 *   * iovec for possible padding after the items
-	 *
-	 * Make sure to update @reserved_parts if you add more parts here.
 	 */
 
-	kdbus_staging_reserve(staging); /* msg.size */
-	kdbus_staging_reserve(staging); /* msg (minus msg.size) plus items */
-	kdbus_staging_reserve(staging); /* msg padding */
-	kdbus_staging_reserve(staging); /* meta */
-	kdbus_staging_reserve(staging); /* meta padding */
-
+	kdbus_staging_reserve(staging, reserved_parts);
 	return staging;
 
 error:
@@ -814,7 +808,7 @@ struct kdbus_staging *kdbus_staging_new_user(struct kdbus_bus *bus,
 	 */
 
 	ret = kdbus_staging_import(staging); /* payload */
-	kdbus_staging_reserve(staging); /* payload padding */
+	kdbus_staging_reserve(staging, reserved_parts); /* payload padding */
 
 	if (ret < 0)
 		goto error;
-- 
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]

#1242259 — [PATCH 35/44] kdbus: Drop goto from kdbus_queue_entry_link()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 35/44] kdbus: Drop goto from kdbus_queue_entry_link()
Message-ID<qhhIg-2d4-85@gated-at.bofh.it>
In reply to#1242233
Use conventional if-else logic instead of goto, which makes the function
easier to read.

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

diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
index 3c0fb3bb55da..5158f00e7148 100644
--- a/ipc/kdbus/queue.c
+++ b/ipc/kdbus/queue.c
@@ -87,6 +87,7 @@ struct kdbus_queue_entry *kdbus_queue_peek(struct kdbus_queue *queue,
 static void kdbus_queue_entry_link(struct kdbus_queue_entry *entry)
 {
 	struct kdbus_queue *queue = &entry->conn->queue;
+	struct kdbus_queue_entry *e;
 	struct rb_node **n, *pn = NULL;
 	bool highest = true;
 
@@ -97,16 +98,11 @@ static void kdbus_queue_entry_link(struct kdbus_queue_entry *entry)
 	/* sort into priority entry tree */
 	n = &queue->msg_prio_queue.rb_node;
 	while (*n) {
-		struct kdbus_queue_entry *e;
-
 		pn = *n;
 		e = rb_entry(pn, struct kdbus_queue_entry, prio_node);
 
-		/* existing node for this priority, add to its list */
-		if (likely(entry->priority == e->priority)) {
-			list_add_tail(&entry->prio_entry, &e->prio_entry);
-			goto prio_done;
-		}
+		if (likely(entry->priority == e->priority))
+			break;
 
 		if (entry->priority < e->priority) {
 			n = &pn->rb_left;
@@ -116,16 +112,20 @@ static void kdbus_queue_entry_link(struct kdbus_queue_entry *entry)
 		}
 	}
 
-	/* cache highest-priority entry */
-	if (highest)
-		queue->msg_prio_highest = &entry->prio_node;
-
-	/* new node for this priority */
-	rb_link_node(&entry->prio_node, pn, n);
-	rb_insert_color(&entry->prio_node, &queue->msg_prio_queue);
-	INIT_LIST_HEAD(&entry->prio_entry);
+	if (*n) {
+		/* existing node for this priority, add to its list */
+		list_add_tail(&entry->prio_entry, &e->prio_entry);
+	} else {
+		/* cache highest-priority entry */
+		if (highest)
+			queue->msg_prio_highest = &entry->prio_node;
+
+		/* new node for this priority */
+		rb_link_node(&entry->prio_node, pn, n);
+		rb_insert_color(&entry->prio_node, &queue->msg_prio_queue);
+		INIT_LIST_HEAD(&entry->prio_entry);
+	}
 
-prio_done:
 	/* add to unsorted fifo list */
 	list_add_tail(&entry->entry, &queue->msg_list);
 }
-- 
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]

#1242260 — [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages()
Message-ID<qhhIg-2d4-83@gated-at.bofh.it>
In reply to#1242233
 - Move `r' and `ret' to scopes where they are used. Drop redundant
   initialization of `ret'.

 - Initialize `bus' on declaration.

 - Replace list_for_each_entry_safe() with list_for_each_entry() when
   iterating over list of replies.

 - Drop redundant `continue'.

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

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index b32b4f981618..6ee688d3de53 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -1293,25 +1293,24 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
 			      u64 name_id)
 {
 	struct kdbus_queue_entry *e, *e_tmp;
-	struct kdbus_reply *r, *r_tmp;
-	struct kdbus_bus *bus;
+	struct kdbus_bus *bus = conn_src->ep->bus;
 	struct kdbus_conn *c;
 	LIST_HEAD(msg_list);
-	int i, ret = 0;
+	int i;
 
 	if (WARN_ON(conn_src == conn_dst))
 		return;
 
-	bus = conn_src->ep->bus;
-
 	/* lock order: domain -> bus -> ep -> names -> conn */
 	down_read(&bus->conn_rwlock);
 	hash_for_each(bus->conn_hash, i, c, hentry) {
+		struct kdbus_reply *r;
+
 		if (c == conn_src || c == conn_dst)
 			continue;
 
 		mutex_lock(&c->lock);
-		list_for_each_entry_safe(r, r_tmp, &c->reply_list, entry) {
+		list_for_each_entry(r, &c->reply_list, entry) {
 			if (r->reply_src != conn_src)
 				continue;
 
@@ -1328,6 +1327,8 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
 
 	kdbus_conn_lock2(conn_src, conn_dst);
 	list_for_each_entry_safe(e, e_tmp, &conn_src->queue.msg_list, entry) {
+		int ret;
+
 		/* filter messages for a specific name */
 		if (name_id > 0 && e->dst_name_id != name_id)
 			continue;
@@ -1343,7 +1344,6 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
 		if (ret < 0) {
 			kdbus_conn_lost_message(conn_dst);
 			kdbus_queue_entry_free(e);
-			continue;
 		}
 	}
 	kdbus_conn_unlock2(conn_src, conn_dst);
-- 
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]

#1242481 — Re: [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 17:00 +0200
SubjectRe: [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages()
Message-ID<qhkPN-6FW-41@gated-at.bofh.it>
In reply to#1242260
Hi

On Thu, Oct 8, 2015 at 1:32 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
>  - Move `r' and `ret' to scopes where they are used. Drop redundant
>    initialization of `ret'.
>
>  - Initialize `bus' on declaration.
>
>  - Replace list_for_each_entry_safe() with list_for_each_entry() when
>    iterating over list of replies.
>
>  - Drop redundant `continue'.
>
> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/connection.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index b32b4f981618..6ee688d3de53 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -1293,25 +1293,24 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
>                               u64 name_id)
>  {
>         struct kdbus_queue_entry *e, *e_tmp;
> -       struct kdbus_reply *r, *r_tmp;
> -       struct kdbus_bus *bus;
> +       struct kdbus_bus *bus = conn_src->ep->bus;
>         struct kdbus_conn *c;
>         LIST_HEAD(msg_list);
> -       int i, ret = 0;
> +       int i;
>
>         if (WARN_ON(conn_src == conn_dst))
>                 return;
>
> -       bus = conn_src->ep->bus;
> -
>         /* lock order: domain -> bus -> ep -> names -> conn */
>         down_read(&bus->conn_rwlock);
>         hash_for_each(bus->conn_hash, i, c, hentry) {
> +               struct kdbus_reply *r;
> +

Why make 'r' local?

>                 if (c == conn_src || c == conn_dst)
>                         continue;
>
>                 mutex_lock(&c->lock);
> -               list_for_each_entry_safe(r, r_tmp, &c->reply_list, entry) {
> +               list_for_each_entry(r, &c->reply_list, entry) {

Looks good.

>                         if (r->reply_src != conn_src)
>                                 continue;
>
> @@ -1328,6 +1327,8 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
>
>         kdbus_conn_lock2(conn_src, conn_dst);
>         list_for_each_entry_safe(e, e_tmp, &conn_src->queue.msg_list, entry) {
> +               int ret;
> +

Why make it local?

>                 /* filter messages for a specific name */
>                 if (name_id > 0 && e->dst_name_id != name_id)
>                         continue;
> @@ -1343,7 +1344,6 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
>                 if (ret < 0) {
>                         kdbus_conn_lost_message(conn_dst);
>                         kdbus_queue_entry_free(e);
> -                       continue;

Looks good.

Thanks
David

>                 }
>         }
>         kdbus_conn_unlock2(conn_src, conn_dst);
> --
> 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]

#1243600 — Re: [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages()

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

On Thu, Oct 08, 2015 at 04:50:31PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Oct 8, 2015 at 1:32 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> >  - Move `r' and `ret' to scopes where they are used. Drop redundant
> >    initialization of `ret'.
> >
> >  - Initialize `bus' on declaration.
> >
> >  - Replace list_for_each_entry_safe() with list_for_each_entry() when
> >    iterating over list of replies.
> >
> >  - Drop redundant `continue'.
> >
> > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> > ---
> >  ipc/kdbus/connection.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> > index b32b4f981618..6ee688d3de53 100644
> > --- a/ipc/kdbus/connection.c
> > +++ b/ipc/kdbus/connection.c
> > @@ -1293,25 +1293,24 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
> >                               u64 name_id)
> >  {
> >         struct kdbus_queue_entry *e, *e_tmp;
> > -       struct kdbus_reply *r, *r_tmp;
> > -       struct kdbus_bus *bus;
> > +       struct kdbus_bus *bus = conn_src->ep->bus;
> >         struct kdbus_conn *c;
> >         LIST_HEAD(msg_list);
> > -       int i, ret = 0;
> > +       int i;
> >
> >         if (WARN_ON(conn_src == conn_dst))
> >                 return;
> >
> > -       bus = conn_src->ep->bus;
> > -
> >         /* lock order: domain -> bus -> ep -> names -> conn */
> >         down_read(&bus->conn_rwlock);
> >         hash_for_each(bus->conn_hash, i, c, hentry) {
> > +               struct kdbus_reply *r;
> > +
> 
> Why make 'r' local?

So what is the rule to keep some vars block-local and some not? This
function has a number of local vars, so the rationale is to keep things
simple. We don't use `r' function wide, but use it only in this block,
so why don't have it block-local?

> 
> >                 if (c == conn_src || c == conn_dst)
> >                         continue;
> >
> >                 mutex_lock(&c->lock);
> > -               list_for_each_entry_safe(r, r_tmp, &c->reply_list, entry) {
> > +               list_for_each_entry(r, &c->reply_list, entry) {
> 
> Looks good.
> 
> >                         if (r->reply_src != conn_src)
> >                                 continue;
> >
> > @@ -1328,6 +1327,8 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
> >
> >         kdbus_conn_lock2(conn_src, conn_dst);
> >         list_for_each_entry_safe(e, e_tmp, &conn_src->queue.msg_list, entry) {
> > +               int ret;
> > +
> 
> Why make it local?

Same thing here. We have the only use of this `ret' var, so why don't
keep it right here?

> 
> >                 /* filter messages for a specific name */
> >                 if (name_id > 0 && e->dst_name_id != name_id)
> >                         continue;
> > @@ -1343,7 +1344,6 @@ void kdbus_conn_move_messages(struct kdbus_conn *conn_dst,
> >                 if (ret < 0) {
> >                         kdbus_conn_lost_message(conn_dst);
> >                         kdbus_queue_entry_free(e);
> > -                       continue;
> 
> Looks good.
> 
> Thanks
> David
> 
> >                 }
> >         }
> >         kdbus_conn_unlock2(conn_src, conn_dst);
> > --
> > 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]

#1242262 — [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:50 +0200
Subject[PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send()
Message-ID<qhhRT-2oG-9@gated-at.bofh.it>
In reply to#1242233
Rearrange tests a little to make them look cleaner.

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

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 93da7f539f74..a4d7414ecaea 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -1977,7 +1977,7 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
 	ret = kdbus_args_parse(&args, argp, &cmd);
 	if (ret < 0)
 		goto exit;
-	else if (ret > 0 && !cmd->msg_address) /* negotiation without msg */
+	if (ret > 0 && !cmd->msg_address) /* negotiation without msg */
 		goto exit;
 
 	ret2 = kdbus_args_parse_msg(&msg_args, KDBUS_PTR(cmd->msg_address),
@@ -1985,10 +1985,11 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
 	if (ret2 < 0) { /* cannot parse message */
 		ret = ret2;
 		goto exit;
-	} else if (ret2 > 0 && !ret) { /* msg-negot implies cmd-negot */
-		ret = -EINVAL;
+	}
+	if (ret > 0) /* negotiation */
 		goto exit;
-	} else if (ret > 0) { /* negotiation */
+	if (ret2 > 0) { /* msg-negot implies cmd-negot */
+		ret = -EINVAL;
 		goto exit;
 	}
 
-- 
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]

#1242429 — Re: [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:40 +0200
SubjectRe: [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send()
Message-ID<qhkwp-6iy-5@gated-at.bofh.it>
In reply to#1242262
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> Rearrange tests a little to make them look cleaner.

What's wrong with 'else if'?

Thanks
David

> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/connection.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 93da7f539f74..a4d7414ecaea 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -1977,7 +1977,7 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
>         ret = kdbus_args_parse(&args, argp, &cmd);
>         if (ret < 0)
>                 goto exit;
> -       else if (ret > 0 && !cmd->msg_address) /* negotiation without msg */
> +       if (ret > 0 && !cmd->msg_address) /* negotiation without msg */
>                 goto exit;
>
>         ret2 = kdbus_args_parse_msg(&msg_args, KDBUS_PTR(cmd->msg_address),
> @@ -1985,10 +1985,11 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
>         if (ret2 < 0) { /* cannot parse message */
>                 ret = ret2;
>                 goto exit;
> -       } else if (ret2 > 0 && !ret) { /* msg-negot implies cmd-negot */
> -               ret = -EINVAL;
> +       }
> +       if (ret > 0) /* negotiation */
>                 goto exit;
> -       } else if (ret > 0) { /* negotiation */
> +       if (ret2 > 0) { /* msg-negot implies cmd-negot */
> +               ret = -EINVAL;
>                 goto exit;
>         }
>
> --
> 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]

#1243574 — Re: [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-09 20:10 +0200
SubjectRe: [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send()
Message-ID<qhKhc-1jS-3@gated-at.bofh.it>
In reply to#1242429
On Thu, Oct 08, 2015 at 04:30:59PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> > Rearrange tests a little to make them look cleaner.
> 
> What's wrong with 'else if'?

Less verbose constructions are easier for our brain to accept :)
Moreover, we usually don't write code in the 'else' branch of
if-statement which handles an error-path.

But I don't mind dropping this patch, it really doesn't save the
universe.

> 
> Thanks
> David
> 
> > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> > ---
> >  ipc/kdbus/connection.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> > index 93da7f539f74..a4d7414ecaea 100644
> > --- a/ipc/kdbus/connection.c
> > +++ b/ipc/kdbus/connection.c
> > @@ -1977,7 +1977,7 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
> >         ret = kdbus_args_parse(&args, argp, &cmd);
> >         if (ret < 0)
> >                 goto exit;
> > -       else if (ret > 0 && !cmd->msg_address) /* negotiation without msg */
> > +       if (ret > 0 && !cmd->msg_address) /* negotiation without msg */
> >                 goto exit;
> >
> >         ret2 = kdbus_args_parse_msg(&msg_args, KDBUS_PTR(cmd->msg_address),
> > @@ -1985,10 +1985,11 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
> >         if (ret2 < 0) { /* cannot parse message */
> >                 ret = ret2;
> >                 goto exit;
> > -       } else if (ret2 > 0 && !ret) { /* msg-negot implies cmd-negot */
> > -               ret = -EINVAL;
> > +       }
> > +       if (ret > 0) /* negotiation */
> >                 goto exit;
> > -       } else if (ret > 0) { /* negotiation */
> > +       if (ret2 > 0) { /* msg-negot implies cmd-negot */
> > +               ret = -EINVAL;
> >                 goto exit;
> >         }
> >
> > --
> > 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 2 of 4 — ← Prev page 1 [2] 3 4  Next page →

Back to top | Article view | linux.kernel

csiph-web