Skip to contentSkip to conversation

kdbus: macros fixes

PATCH In review Sergei Zviagintsev 3 participants 24 messages Jun 9, 2015 – Jun 19, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover kdbus: macros fixes

Fix type conversion and style issues in item macros.

First submission was in 2 separate emails:
https://lkml.kernel.org/g/[email protected]
https://lkml.kernel.org/g/[email protected]

Changes in v2:

- fixed similar type cast issue in KDBUS_ITEMS_END macro
- all arguments in iteration macros are enclosed in parentheses, instead of using loop cursor without them
- summary phrases and commit messages updated
- documentation updates are excluded to their own patches
- added fixes for samples/kdbus
- added fixes for selftests/kdbus

Sergei Zviagintsev (10):
kdbus: fix operator precedence issues in item macros
kdbus: use parentheses uniformly in KDBUS_ITEMS_FOREACH macro Documentation/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT
macro
Documentation/kdbus: use parentheses uniformly in KDBUS_ITEM_FOREACH
macro
selftests/kdbus: fix trivial style issues
selftests/kdbus: fix precedence issues in macros selftests/kdbus: use parentheses in iteration macros uniformly samples/kdbus: add whitespace
samples/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT macro samples/kdbus: use parentheses uniformly in KDBUS_FOREACH macro

 Documentation/kdbus/kdbus.item.xml         |  6 +++---
 ipc/kdbus/item.h                           | 10 ++++-----
 samples/kdbus/kdbus-api.h                  |  6 +++---
 tools/testing/selftests/kdbus/kdbus-enum.h |  1 +
 tools/testing/selftests/kdbus/kdbus-util.c |  2 +-
 tools/testing/selftests/kdbus/kdbus-util.h | 33 ++++++++++++++----------------
 6 files changed, 28 insertions(+), 30 deletions(-)
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH v2 1/10 kdbus: fix operator precedence issues in item macros

_i' argument in KDBUS_ITEM_NEXT and KDBUS_ITEMS_END macros is not enclosed into parentheses when the cast operator is applied, which leads to improper type conversion if _i' is supplied as a complex expression, e.g.

KDBUS_ITEM_NEXT(condition ? a : b)

KDBUS_ITEMS_SIZE macro has similar issue, missing parentheses around `_h' when using indirection operator.

Use parentheses properly to guarantee right precedence.

---
 ipc/kdbus/item.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
ipc/kdbus/item.h +3 -3
index eeefd8beac3b..32909e2e7954 100644
@@ -21,8 +21,8 @@
 #include "util.h"
 
 /* generic access and iterators over a stream of items */
-#define KDBUS_ITEM_NEXT(_i) (typeof(_i))(((u8 *)_i) + KDBUS_ALIGN8((_i)->size))
-#define KDBUS_ITEMS_SIZE(_h, _is) ((_h)->size - offsetof(typeof(*_h), _is))
+#define KDBUS_ITEM_NEXT(_i) (typeof(_i))((u8 *)(_i) + KDBUS_ALIGN8((_i)->size))
+#define KDBUS_ITEMS_SIZE(_h, _is) ((_h)->size - offsetof(typeof(*(_h)), _is))
 #define KDBUS_ITEM_HEADER_SIZE offsetof(struct kdbus_item, data)
 #define KDBUS_ITEM_SIZE(_s) KDBUS_ALIGN8(KDBUS_ITEM_HEADER_SIZE + (_s))
 #define KDBUS_ITEM_PAYLOAD_SIZE(_i) ((_i)->size - KDBUS_ITEM_HEADER_SIZE)
@@ -40,7 +40,7 @@
 	 (u8 *)(_i) >= (u8 *)(_is))
 
 #define KDBUS_ITEMS_END(_i, _is, _s)					\
-	((u8 *)_i == ((u8 *)(_is) + KDBUS_ALIGN8(_s)))
+	((u8 *)(_i) == ((u8 *)(_is) + KDBUS_ALIGN8(_s)))
 
 /**
  * struct kdbus_item_header - Describes the fix part of an item
Signature · 1 line
1.8.3.1
Reviewed-by on v2 1/10
#2 lore raw

Re: [PATCH v2 01/10] kdbus: fix operator precedence issues in item macros

Hi

Quoted 10 lines · Sergei Zviagintsev

_i' argument in KDBUS_ITEM_NEXT and KDBUS_ITEMS_END macros is not enclosed into parentheses when the cast operator is applied, which leads to improper type conversion if _i' is supplied as a complex expression, e.g.

KDBUS_ITEM_NEXT(condition ? a : b)

KDBUS_ITEMS_SIZE macro has similar issue, missing parentheses around `_h' when using indirection operator.

Quoted 6 lines

Use parentheses properly to guarantee right precedence.

---
 ipc/kdbus/item.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Thanks
David

Quoted 27 lines
ipc/kdbus/item.h +3 -4
index eeefd8beac3b..32909e2e7954 100644
@@ -21,8 +21,8 @@
 #include "util.h"
 
 /* generic access and iterators over a stream of items */
-#define KDBUS_ITEM_NEXT(_i) (typeof(_i))(((u8 *)_i) + KDBUS_ALIGN8((_i)->size))
-#define KDBUS_ITEMS_SIZE(_h, _is) ((_h)->size - offsetof(typeof(*_h), _is))
+#define KDBUS_ITEM_NEXT(_i) (typeof(_i))((u8 *)(_i) + KDBUS_ALIGN8((_i)->size))
+#define KDBUS_ITEMS_SIZE(_h, _is) ((_h)->size - offsetof(typeof(*(_h)), _is))
 #define KDBUS_ITEM_HEADER_SIZE offsetof(struct kdbus_item, data)
 #define KDBUS_ITEM_SIZE(_s) KDBUS_ALIGN8(KDBUS_ITEM_HEADER_SIZE + (_s))
 #define KDBUS_ITEM_PAYLOAD_SIZE(_i) ((_i)->size - KDBUS_ITEM_HEADER_SIZE)
@@ -40,7 +40,7 @@
         (u8 *)(_i) >= (u8 *)(_is))
 
 #define KDBUS_ITEMS_END(_i, _is, _s)                                   \
-       ((u8 *)_i == ((u8 *)(_is) + KDBUS_ALIGN8(_s)))
+       ((u8 *)(_i) == ((u8 *)(_is) + KDBUS_ALIGN8(_s)))
 
 /**
  * struct kdbus_item_header - Describes the fix part of an item
--

1.8.3.1

#3 lore raw

PATCH v2 2/10 kdbus: use parentheses uniformly in KDBUS_ITEMS_FOREACH macro

In reply to Sergei Zviagintsev

Enclose all arguments into parentheses to stay consistent across the whole macro.

---
 ipc/kdbus/item.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
ipc/kdbus/item.h +2 -2
index 32909e2e7954..bca63b4e6e80 100644
@@ -28,10 +28,10 @@
 #define KDBUS_ITEM_PAYLOAD_SIZE(_i) ((_i)->size - KDBUS_ITEM_HEADER_SIZE)
 
 #define KDBUS_ITEMS_FOREACH(_i, _is, _s)				\
-	for (_i = _is;							\
+	for ((_i) = (_is);						\
 	     ((u8 *)(_i) < (u8 *)(_is) + (_s)) &&			\
 	       ((u8 *)(_i) >= (u8 *)(_is));				\
-	     _i = KDBUS_ITEM_NEXT(_i))
+	     (_i) = KDBUS_ITEM_NEXT(_i))
 
 #define KDBUS_ITEM_VALID(_i, _is, _s)					\
 	((_i)->size >= KDBUS_ITEM_HEADER_SIZE &&			\
Signature · 1 line
1.8.3.1
Reviewed-by on v2 2/10
#4 lore raw

Re: [PATCH v2 02/10] kdbus: use parentheses uniformly in KDBUS_ITEMS_FOREACH macro

Hi

Quoted 7 lines · Sergei Zviagintsev

Enclose all arguments into parentheses to stay consistent across the whole macro.

---
 ipc/kdbus/item.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Thanks
David

Quoted 20 lines
ipc/kdbus/item.h +2 -3
index 32909e2e7954..bca63b4e6e80 100644
@@ -28,10 +28,10 @@
 #define KDBUS_ITEM_PAYLOAD_SIZE(_i) ((_i)->size - KDBUS_ITEM_HEADER_SIZE)
 
 #define KDBUS_ITEMS_FOREACH(_i, _is, _s)                               \
-       for (_i = _is;                                                  \
+       for ((_i) = (_is);                                              \
             ((u8 *)(_i) < (u8 *)(_is) + (_s)) &&                       \
               ((u8 *)(_i) >= (u8 *)(_is));                             \
-            _i = KDBUS_ITEM_NEXT(_i))
+            (_i) = KDBUS_ITEM_NEXT(_i))
 
 #define KDBUS_ITEM_VALID(_i, _is, _s)                                  \
        ((_i)->size >= KDBUS_ITEM_HEADER_SIZE &&                        \
--

1.8.3.1

#5 lore raw

PATCH v2 3/10 Documentation/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT macro

In reply to Sergei Zviagintsev

item' argument in KDBUS_ITEM_NEXT macro example is not enclosed into parentheses when the cast operator is applied, which leads to improper type conversion if item' is supplied as a complex expression, e.g.

KDBUS_ITEM_NEXT(condition ? a : b)

Use parentheses properly to guarantee right precedence.

---
 Documentation/kdbus/kdbus.item.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/kdbus/kdbus.item.xml +1 -1
index 09f8b903116f..b0eeeef995af 100644
@@ -69,7 +69,7 @@
 #define KDBUS_ALIGN8(val) (((val) + 7) & ~7)
 
 #define KDBUS_ITEM_NEXT(item) \
-    (typeof(item))(((uint8_t *)item) + KDBUS_ALIGN8((item)->size))
+    (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 
 #define KDBUS_ITEM_FOREACH(item, head, first)                      \
     for (item = (head)->first;                                     \
Signature · 1 line
1.8.3.1
Reviewed-by on v2 3/10
#6 lore raw

Re: [PATCH v2 03/10] Documentation/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT macro

Hi

Quoted 4 lines · Sergei Zviagintsev

item' argument in KDBUS_ITEM_NEXT macro example is not enclosed into parentheses when the cast operator is applied, which leads to improper type conversion if item' is supplied as a complex expression, e.g.

Quoted 8 lines

KDBUS_ITEM_NEXT(condition ? a : b)

Use parentheses properly to guarantee right precedence.

---
 Documentation/kdbus/kdbus.item.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks
David

Quoted 16 lines
Documentation/kdbus/kdbus.item.xml +1 -2
index 09f8b903116f..b0eeeef995af 100644
@@ -69,7 +69,7 @@
 #define KDBUS_ALIGN8(val) (((val) + 7) & ~7)
 
 #define KDBUS_ITEM_NEXT(item) \
-    (typeof(item))(((uint8_t *)item) + KDBUS_ALIGN8((item)->size))
+    (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 
 #define KDBUS_ITEM_FOREACH(item, head, first)                      \
     for (item = (head)->first;                                     \
--

1.8.3.1

#7 lore raw

PATCH v2 4/10 Documentation/kdbus: use parentheses uniformly in KDBUS_ITEM_FOREACH macro

In reply to Sergei Zviagintsev

Enclose all arguments into parentheses to stay consistent across the whole macro.

---
 Documentation/kdbus/kdbus.item.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/kdbus/kdbus.item.xml +2 -2
index b0eeeef995af..ee09dfa443b8 100644
@@ -72,10 +72,10 @@
     (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 
 #define KDBUS_ITEM_FOREACH(item, head, first)                      \
-    for (item = (head)->first;                                     \
+    for ((item) = (head)->first;                                   \
          ((uint8_t *)(item) < (uint8_t *)(head) + (head)->size) && \
           ((uint8_t *)(item) >= (uint8_t *)(head));                \
-         item = KDBUS_ITEM_NEXT(item))
+         (item) = KDBUS_ITEM_NEXT(item))
       ]]></programlisting>
     </refsect2>
   </refsect1>
Signature · 1 line
1.8.3.1
Reviewed-by on v2 4/10
#8 lore raw

Re: [PATCH v2 04/10] Documentation/kdbus: use parentheses uniformly in KDBUS_ITEM_FOREACH macro

Hi

Quoted 13 lines · Sergei Zviagintsev

Enclose all arguments into parentheses to stay consistent across the whole macro.

---
 Documentation/kdbus/kdbus.item.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/kdbus/kdbus.item.xml +0 -0
index b0eeeef995af..ee09dfa443b8 100644

Documentation/kdbus/kdbus.item.xml line 72

Quoted 1 line
Documentation/kdbus/kdbus.item.xml +0 -0
@@ -72,10 +72,10 @@
     (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))

Thanks
David

Quoted 13 lines
Documentation/kdbus/kdbus.item.xml +2 -3
 #define KDBUS_ITEM_FOREACH(item, head, first)                      \
-    for (item = (head)->first;                                     \
+    for ((item) = (head)->first;                                   \
          ((uint8_t *)(item) < (uint8_t *)(head) + (head)->size) && \
           ((uint8_t *)(item) >= (uint8_t *)(head));                \
-         item = KDBUS_ITEM_NEXT(item))
+         (item) = KDBUS_ITEM_NEXT(item))
       ]]></programlisting>
     </refsect2>
   </refsect1>
--

1.8.3.1

#9 lore raw

PATCH v2 5/10 selftests/kdbus: fix trivial style issues

In reply to Sergei Zviagintsev
---
 tools/testing/selftests/kdbus/kdbus-enum.h |  1 +
 tools/testing/selftests/kdbus/kdbus-util.c |  2 +-
 tools/testing/selftests/kdbus/kdbus-util.h | 21 +++++++++------------
 3 files changed, 11 insertions(+), 13 deletions(-)
tools/testing/selftests/kdbus/kdbus-enum.h +1 -0
index a67cec3512a7..ed28cca26906 100644
@@ -6,6 +6,7 @@
  * Free Software Foundation; either version 2.1 of the License, or (at
  * your option) any later version.
  */
+
 #pragma once
 
 const char *enum_CMD(long long id);
tools/testing/selftests/kdbus/kdbus-util.c +1 -1
index 4b376ecfdbed..1e267d585a14 100644
@@ -1353,7 +1353,7 @@ static int all_ids_are_mapped(const char *path)
 	return 0;
 }
 
-int all_uids_gids_are_mapped()
+int all_uids_gids_are_mapped(void)
 {
 	int ret;
 
tools/testing/selftests/kdbus/kdbus-util.h +9 -12
index 50ff07140bdd..b53b03f0565c 100644
@@ -7,6 +7,7 @@
  * Free Software Foundation; either version 2.1 of the License, or (at
  * your option) any later version.
  */
+
 #pragma once
 
 #define BIT(X) (1 << (X))
@@ -30,24 +31,22 @@
 #define KDBUS_ITEM_FOREACH(item, head, first)				\
 	for (item = (head)->first;					\
 	     ((uint8_t *)(item) < (uint8_t *)(head) + (head)->size) &&	\
-	       ((uint8_t *)(item) >= (uint8_t *)(head));	\
+	       ((uint8_t *)(item) >= (uint8_t *)(head));		\
 	     item = KDBUS_ITEM_NEXT(item))
 #define KDBUS_FOREACH(iter, first, _size)				\
 	for (iter = (first);						\
 	     ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&	\
 	       ((uint8_t *)(iter) >= (uint8_t *)(first));		\
-	     iter = (void*)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
-
+	     iter = (void *)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
 
-#define _KDBUS_ATTACH_BITS_SET_NR  (__builtin_popcountll(_KDBUS_ATTACH_ALL))
+#define _KDBUS_ATTACH_BITS_SET_NR (__builtin_popcountll(_KDBUS_ATTACH_ALL))
 
 /* Sum of KDBUS_ITEM_* that reflects _KDBUS_ATTACH_ALL */
-#define KDBUS_ATTACH_ITEMS_TYPE_SUM \
-	((((_KDBUS_ATTACH_BITS_SET_NR - 1) * \
-	((_KDBUS_ATTACH_BITS_SET_NR - 1) + 1)) / 2 ) + \
+#define KDBUS_ATTACH_ITEMS_TYPE_SUM					\
+	((((_KDBUS_ATTACH_BITS_SET_NR - 1) *				\
+	((_KDBUS_ATTACH_BITS_SET_NR - 1) + 1)) / 2) +			\
 	(_KDBUS_ITEM_ATTACH_BASE * _KDBUS_ATTACH_BITS_SET_NR))
 
-
 #define POOL_SIZE (16 * 1024LU * 1024LU)
 
 #define UNPRIV_UID 65534
@@ -207,14 +206,12 @@ int kdbus_add_match_id(struct kdbus_conn *conn, uint64_t cookie,
 		       uint64_t type, uint64_t id);
 int kdbus_add_match_empty(struct kdbus_conn *conn);
 
-int all_uids_gids_are_mapped();
+int all_uids_gids_are_mapped(void);
 int drop_privileges(uid_t uid, gid_t gid);
 uint64_t now(clockid_t clock);
 char *unique_name(const char *prefix);
 
-int userns_map_uid_gid(pid_t pid,
-		       const char *map_uid,
-		       const char *map_gid);
+int userns_map_uid_gid(pid_t pid, const char *map_uid, const char *map_gid);
 int test_is_capable(int cap, ...);
 int config_user_ns_is_enabled(void);
 int config_auditsyscall_is_enabled(void);
Signature · 1 line
1.8.3.1
Reviewed-by on v2 5/10
#10 lore raw

Re: [PATCH v2 05/10] selftests/kdbus: fix trivial style issues

Hi

Quoted 6 lines · Sergei Zviagintsev
---
 tools/testing/selftests/kdbus/kdbus-enum.h |  1 +
 tools/testing/selftests/kdbus/kdbus-util.c |  2 +-
 tools/testing/selftests/kdbus/kdbus-util.h | 21 +++++++++------------
 3 files changed, 11 insertions(+), 13 deletions(-)

Thanks
David

Quoted 88 lines
tools/testing/selftests/kdbus/kdbus-enum.h +1 -0
index a67cec3512a7..ed28cca26906 100644
@@ -6,6 +6,7 @@
  * Free Software Foundation; either version 2.1 of the License, or (at
  * your option) any later version.
  */
+
 #pragma once
 
 const char *enum_CMD(long long id);
tools/testing/selftests/kdbus/kdbus-util.c +1 -1
index 4b376ecfdbed..1e267d585a14 100644
@@ -1353,7 +1353,7 @@ static int all_ids_are_mapped(const char *path)
        return 0;
 }
 
-int all_uids_gids_are_mapped()
+int all_uids_gids_are_mapped(void)
 {
        int ret;
 
tools/testing/selftests/kdbus/kdbus-util.h +9 -13
index 50ff07140bdd..b53b03f0565c 100644
@@ -7,6 +7,7 @@
  * Free Software Foundation; either version 2.1 of the License, or (at
  * your option) any later version.
  */
+
 #pragma once
 
 #define BIT(X) (1 << (X))
@@ -30,24 +31,22 @@
 #define KDBUS_ITEM_FOREACH(item, head, first)                          \
        for (item = (head)->first;                                      \
             ((uint8_t *)(item) < (uint8_t *)(head) + (head)->size) &&  \
-              ((uint8_t *)(item) >= (uint8_t *)(head));        \
+              ((uint8_t *)(item) >= (uint8_t *)(head));                \
             item = KDBUS_ITEM_NEXT(item))
 #define KDBUS_FOREACH(iter, first, _size)                              \
        for (iter = (first);                                            \
             ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&      \
               ((uint8_t *)(iter) >= (uint8_t *)(first));               \
-            iter = (void*)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
-
+            iter = (void *)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
 
-#define _KDBUS_ATTACH_BITS_SET_NR  (__builtin_popcountll(_KDBUS_ATTACH_ALL))
+#define _KDBUS_ATTACH_BITS_SET_NR (__builtin_popcountll(_KDBUS_ATTACH_ALL))
 
 /* Sum of KDBUS_ITEM_* that reflects _KDBUS_ATTACH_ALL */
-#define KDBUS_ATTACH_ITEMS_TYPE_SUM \
-       ((((_KDBUS_ATTACH_BITS_SET_NR - 1) * \
-       ((_KDBUS_ATTACH_BITS_SET_NR - 1) + 1)) / 2 ) + \
+#define KDBUS_ATTACH_ITEMS_TYPE_SUM                                    \
+       ((((_KDBUS_ATTACH_BITS_SET_NR - 1) *                            \
+       ((_KDBUS_ATTACH_BITS_SET_NR - 1) + 1)) / 2) +                   \
        (_KDBUS_ITEM_ATTACH_BASE * _KDBUS_ATTACH_BITS_SET_NR))
 
-
 #define POOL_SIZE (16 * 1024LU * 1024LU)
 
 #define UNPRIV_UID 65534
@@ -207,14 +206,12 @@ int kdbus_add_match_id(struct kdbus_conn *conn, uint64_t cookie,
                       uint64_t type, uint64_t id);
 int kdbus_add_match_empty(struct kdbus_conn *conn);
 
-int all_uids_gids_are_mapped();
+int all_uids_gids_are_mapped(void);
 int drop_privileges(uid_t uid, gid_t gid);
 uint64_t now(clockid_t clock);
 char *unique_name(const char *prefix);
 
-int userns_map_uid_gid(pid_t pid,
-                      const char *map_uid,
-                      const char *map_gid);
+int userns_map_uid_gid(pid_t pid, const char *map_uid, const char *map_gid);
 int test_is_capable(int cap, ...);
 int config_user_ns_is_enabled(void);
 int config_auditsyscall_is_enabled(void);
--

1.8.3.1

#11 lore raw

PATCH v2 6/10 selftests/kdbus: fix precedence issues in macros

In reply to Sergei Zviagintsev

item' argument in KDBUS_ITEM_NEXT macro is not enclosed into parentheses when the cast operator is applied, which leads to improper type conversion if item' is supplied as a complex expression, e.g.

KDBUS_ITEM_NEXT(condition ? a : b)

RUN_CLONE_CHILD macro has similar issue, missing parentheses around `clone_ret' when using indirection operator.

Use parentheses properly to guarantee right precedence.

---
 tools/testing/selftests/kdbus/kdbus-util.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/testing/selftests/kdbus/kdbus-util.h +3 -3
index b53b03f0565c..df5721ee8f54 100644
@@ -27,7 +27,7 @@
 #define KDBUS_ITEM_SIZE(s) KDBUS_ALIGN8((s) + KDBUS_ITEM_HEADER_SIZE)
 
 #define KDBUS_ITEM_NEXT(item) \
-	(typeof(item))(((uint8_t *)item) + KDBUS_ALIGN8((item)->size))
+	(typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 #define KDBUS_ITEM_FOREACH(item, head, first)				\
 	for (item = (head)->first;					\
 	     ((uint8_t *)(item) < (uint8_t *)(head) + (head)->size) &&	\
@@ -104,7 +104,7 @@ extern int kdbus_util_verbose;
 	_setup_;							\
 	efd = eventfd(0, EFD_CLOEXEC);					\
 	ASSERT_RETURN(efd >= 0);					\
-	*clone_ret = 0;							\
+	*(clone_ret) = 0;						\
 	pid = syscall(__NR_clone, flags, NULL);				\
 	if (pid == 0) {							\
 		eventfd_t event_status = 0;				\
@@ -129,7 +129,7 @@ extern int kdbus_util_verbose;
 		ret = TEST_OK;						\
 	} else {							\
 		ret = -errno;						\
-		*clone_ret = -errno;					\
+		*(clone_ret) = -errno;					\
 	}								\
 	close(efd);							\
 	ret;								\
Signature · 1 line
1.8.3.1
Reviewed-by on v2 6/10
#12 lore raw

Re: [PATCH v2 06/10] selftests/kdbus: fix precedence issues in macros

Hi

Quoted 9 lines · Sergei Zviagintsev

item' argument in KDBUS_ITEM_NEXT macro is not enclosed into parentheses when the cast operator is applied, which leads to improper type conversion if item' is supplied as a complex expression, e.g.

KDBUS_ITEM_NEXT(condition ? a : b)

RUN_CLONE_CHILD macro has similar issue, missing parentheses around `clone_ret' when using indirection operator.

Quoted 6 lines

Use parentheses properly to guarantee right precedence.

---
 tools/testing/selftests/kdbus/kdbus-util.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Thanks
David

Quoted 34 lines
tools/testing/selftests/kdbus/kdbus-util.h +3 -4
index b53b03f0565c..df5721ee8f54 100644
@@ -27,7 +27,7 @@
 #define KDBUS_ITEM_SIZE(s) KDBUS_ALIGN8((s) + KDBUS_ITEM_HEADER_SIZE)
 
 #define KDBUS_ITEM_NEXT(item) \
-       (typeof(item))(((uint8_t *)item) + KDBUS_ALIGN8((item)->size))
+       (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 #define KDBUS_ITEM_FOREACH(item, head, first)                          \
        for (item = (head)->first;                                      \
             ((uint8_t *)(item) < (uint8_t *)(head) + (head)->size) &&  \
@@ -104,7 +104,7 @@ extern int kdbus_util_verbose;
        _setup_;                                                        \
        efd = eventfd(0, EFD_CLOEXEC);                                  \
        ASSERT_RETURN(efd >= 0);                                        \
-       *clone_ret = 0;                                                 \
+       *(clone_ret) = 0;                                               \
        pid = syscall(__NR_clone, flags, NULL);                         \
        if (pid == 0) {                                                 \
                eventfd_t event_status = 0;                             \
@@ -129,7 +129,7 @@ extern int kdbus_util_verbose;
                ret = TEST_OK;                                          \
        } else {                                                        \
                ret = -errno;                                           \
-               *clone_ret = -errno;                                    \
+               *(clone_ret) = -errno;                                  \
        }                                                               \
        close(efd);                                                     \
        ret;                                                            \
--

1.8.3.1

#13 lore raw

PATCH v2 7/10 selftests/kdbus: use parentheses in iteration macros uniformly

In reply to Sergei Zviagintsev

Enclose all arguments into parentheses in KDBUS_ITEM_FOREACH and KDBUS_FOREACH macros to stay consistent across the whole macro.

---
 tools/testing/selftests/kdbus/kdbus-util.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
tools/testing/selftests/kdbus/kdbus-util.h +4 -4
index df5721ee8f54..d1a0f1b4d0eb 100644
@@ -29,15 +29,15 @@
 #define KDBUS_ITEM_NEXT(item) \
 	(typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 #define KDBUS_ITEM_FOREACH(item, head, first)				\
-	for (item = (head)->first;					\
+	for ((item) = (head)->first;					\
 	     ((uint8_t *)(item) < (uint8_t *)(head) + (head)->size) &&	\
 	       ((uint8_t *)(item) >= (uint8_t *)(head));		\
-	     item = KDBUS_ITEM_NEXT(item))
+	     (item) = KDBUS_ITEM_NEXT(item))
 #define KDBUS_FOREACH(iter, first, _size)				\
-	for (iter = (first);						\
+	for ((iter) = (first);						\
 	     ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&	\
 	       ((uint8_t *)(iter) >= (uint8_t *)(first));		\
-	     iter = (void *)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
+	     (iter) = (void *)((uint8_t *)(iter) + KDBUS_ALIGN8((iter)->size)))
 
 #define _KDBUS_ATTACH_BITS_SET_NR (__builtin_popcountll(_KDBUS_ATTACH_ALL))
 
Signature · 1 line
1.8.3.1
Reviewed-by on v2 7/10
#14 lore raw

Re: [PATCH v2 07/10] selftests/kdbus: use parentheses in iteration macros uniformly

Hi

Quoted 7 lines · Sergei Zviagintsev

Enclose all arguments into parentheses in KDBUS_ITEM_FOREACH and KDBUS_FOREACH macros to stay consistent across the whole macro.

---
 tools/testing/selftests/kdbus/kdbus-util.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Thanks
David

Quoted 27 lines
tools/testing/selftests/kdbus/kdbus-util.h +4 -5
index df5721ee8f54..d1a0f1b4d0eb 100644
@@ -29,15 +29,15 @@
 #define KDBUS_ITEM_NEXT(item) \
        (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 #define KDBUS_ITEM_FOREACH(item, head, first)                          \
-       for (item = (head)->first;                                      \
+       for ((item) = (head)->first;                                    \
             ((uint8_t *)(item) < (uint8_t *)(head) + (head)->size) &&  \
               ((uint8_t *)(item) >= (uint8_t *)(head));                \
-            item = KDBUS_ITEM_NEXT(item))
+            (item) = KDBUS_ITEM_NEXT(item))
 #define KDBUS_FOREACH(iter, first, _size)                              \
-       for (iter = (first);                                            \
+       for ((iter) = (first);                                          \
             ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&      \
               ((uint8_t *)(iter) >= (uint8_t *)(first));               \
-            iter = (void *)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
+            (iter) = (void *)((uint8_t *)(iter) + KDBUS_ALIGN8((iter)->size)))
 
 #define _KDBUS_ATTACH_BITS_SET_NR (__builtin_popcountll(_KDBUS_ATTACH_ALL))
 
--

1.8.3.1

#15 lore raw

PATCH v2 8/10 samples/kdbus: add whitespace

In reply to Sergei Zviagintsev
---
 samples/kdbus/kdbus-api.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
samples/kdbus/kdbus-api.h +1 -1
index 5ed5907c5cb4..2de4d6a8c51e 100644
@@ -13,7 +13,7 @@
 	for (iter = (first);						\
 	     ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&	\
 	       ((uint8_t *)(iter) >= (uint8_t *)(first));		\
-	     iter = (void*)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
+	     iter = (void *)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
 
 static inline int kdbus_cmd_bus_make(int control_fd, struct kdbus_cmd *cmd)
 {
Signature · 1 line
1.8.3.1
Reviewed-by on v2 8/10
#16 lore raw

Re: [PATCH v2 08/10] samples/kdbus: add whitespace

Hi

Quoted 4 lines · Sergei Zviagintsev
---
 samples/kdbus/kdbus-api.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks
David

Quoted 16 lines
samples/kdbus/kdbus-api.h +1 -2
index 5ed5907c5cb4..2de4d6a8c51e 100644
@@ -13,7 +13,7 @@
        for (iter = (first);                                            \
             ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&      \
               ((uint8_t *)(iter) >= (uint8_t *)(first));               \
-            iter = (void*)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
+            iter = (void *)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
 
 static inline int kdbus_cmd_bus_make(int control_fd, struct kdbus_cmd *cmd)
 {
--

1.8.3.1

#17 lore raw

PATCH v2 9/10 samples/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT macro

In reply to Sergei Zviagintsev

item' argument in KDBUS_ITEM_NEXT macro is not enclosed into parentheses when the cast operator is applied, which leads to improper type conversion if item' is supplied as a complex expression, e.g.

KDBUS_ITEM_NEXT(condition ? a : b)

Use parentheses properly to guarantee right precedence.

---
 samples/kdbus/kdbus-api.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
samples/kdbus/kdbus-api.h +1 -1
index 2de4d6a8c51e..fab873b89d97 100644
@@ -8,7 +8,7 @@
 #define KDBUS_ITEM_HEADER_SIZE offsetof(struct kdbus_item, data)
 #define KDBUS_ITEM_SIZE(s) KDBUS_ALIGN8((s) + KDBUS_ITEM_HEADER_SIZE)
 #define KDBUS_ITEM_NEXT(item) \
-	(typeof(item))(((uint8_t *)item) + KDBUS_ALIGN8((item)->size))
+	(typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 #define KDBUS_FOREACH(iter, first, _size)				\
 	for (iter = (first);						\
 	     ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&	\
Signature · 1 line
1.8.3.1
Reviewed-by on v2 9/10
#18 lore raw

Re: [PATCH v2 09/10] samples/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT macro

Hi

Quoted 4 lines · Sergei Zviagintsev

item' argument in KDBUS_ITEM_NEXT macro is not enclosed into parentheses when the cast operator is applied, which leads to improper type conversion if item' is supplied as a complex expression, e.g.

Quoted 8 lines

KDBUS_ITEM_NEXT(condition ? a : b)

Use parentheses properly to guarantee right precedence.

---
 samples/kdbus/kdbus-api.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks
David

Quoted 16 lines
samples/kdbus/kdbus-api.h +1 -2
index 2de4d6a8c51e..fab873b89d97 100644
@@ -8,7 +8,7 @@
 #define KDBUS_ITEM_HEADER_SIZE offsetof(struct kdbus_item, data)
 #define KDBUS_ITEM_SIZE(s) KDBUS_ALIGN8((s) + KDBUS_ITEM_HEADER_SIZE)
 #define KDBUS_ITEM_NEXT(item) \
-       (typeof(item))(((uint8_t *)item) + KDBUS_ALIGN8((item)->size))
+       (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 #define KDBUS_FOREACH(iter, first, _size)                              \
        for (iter = (first);                                            \
             ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&      \
--

1.8.3.1

#19 lore raw

PATCH v2 10/10 samples/kdbus: use parentheses uniformly in KDBUS_FOREACH macro

In reply to Sergei Zviagintsev

Enclose all arguments into parentheses to stay consistent across the whole macro.

---
 samples/kdbus/kdbus-api.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
samples/kdbus/kdbus-api.h +2 -2
index fab873b89d97..7f3abae18396 100644
@@ -10,10 +10,10 @@
 #define KDBUS_ITEM_NEXT(item) \
 	(typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 #define KDBUS_FOREACH(iter, first, _size)				\
-	for (iter = (first);						\
+	for ((iter) = (first);						\
 	     ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&	\
 	       ((uint8_t *)(iter) >= (uint8_t *)(first));		\
-	     iter = (void *)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
+	     (iter) = (void *)((uint8_t *)(iter) + KDBUS_ALIGN8((iter)->size)))
 
 static inline int kdbus_cmd_bus_make(int control_fd, struct kdbus_cmd *cmd)
 {
Signature · 1 line
1.8.3.1
Reviewed-by on v2 10/10
#20 lore raw

Re: [PATCH v2 10/10] samples/kdbus: use parentheses uniformly in KDBUS_FOREACH macro

Hi

Quoted 7 lines · Sergei Zviagintsev

Enclose all arguments into parentheses to stay consistent across the whole macro.

---
 samples/kdbus/kdbus-api.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Thanks
David

Quoted 20 lines
samples/kdbus/kdbus-api.h +2 -3
index fab873b89d97..7f3abae18396 100644
@@ -10,10 +10,10 @@
 #define KDBUS_ITEM_NEXT(item) \
        (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))
 #define KDBUS_FOREACH(iter, first, _size)                              \
-       for (iter = (first);                                            \
+       for ((iter) = (first);                                          \
             ((uint8_t *)(iter) < (uint8_t *)(first) + (_size)) &&      \
               ((uint8_t *)(iter) >= (uint8_t *)(first));               \
-            iter = (void *)(((uint8_t *)iter) + KDBUS_ALIGN8((iter)->size)))
+            (iter) = (void *)((uint8_t *)(iter) + KDBUS_ALIGN8((iter)->size)))
 
 static inline int kdbus_cmd_bus_make(int control_fd, struct kdbus_cmd *cmd)
 {
--

1.8.3.1

#21 lore raw
In reply to Sergei Zviagintsev

Hi

Quoted 17 lines · Sergei Zviagintsev

Fix type conversion and style issues in item macros.

First submission was in 2 separate emails:
https://lkml.kernel.org/g/[email protected]
https://lkml.kernel.org/g/[email protected]

Changes in v2:

- fixed similar type cast issue in KDBUS_ITEMS_END macro
- all arguments in iteration macros are enclosed in parentheses, instead of using loop cursor without them
- summary phrases and commit messages updated
- documentation updates are excluded to their own patches
- added fixes for samples/kdbus
- added fixes for selftests/kdbus

Quoted 13 lines

Sergei Zviagintsev (10):
kdbus: fix operator precedence issues in item macros
kdbus: use parentheses uniformly in KDBUS_ITEMS_FOREACH macro Documentation/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT
macro
Documentation/kdbus: use parentheses uniformly in KDBUS_ITEM_FOREACH
macro
selftests/kdbus: fix trivial style issues
selftests/kdbus: fix precedence issues in macros selftests/kdbus: use parentheses in iteration macros uniformly samples/kdbus: add whitespace
samples/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT macro samples/kdbus: use parentheses uniformly in KDBUS_FOREACH macro

Next time, please merge most of these changes to reduce the number of patches and review-overhead. But looks all good to me, much appreciated.

Thanks
David

#22 lore raw

Hi

Quoted 3 lines · David Herrmann

Next time, please merge most of these changes to reduce the number of patches and review-overhead. But looks all good to me, much appreciated.

Thank you for response, I will keep that in mind.

#23 lore raw
In reply to Sergei Zviagintsev
Quoted 31 lines · Sergei Zviagintsev

Fix type conversion and style issues in item macros.

First submission was in 2 separate emails:
https://lkml.kernel.org/g/[email protected]
https://lkml.kernel.org/g/[email protected]

Changes in v2:

- fixed similar type cast issue in KDBUS_ITEMS_END macro
- all arguments in iteration macros are enclosed in parentheses, instead of using loop cursor without them
- summary phrases and commit messages updated
- documentation updates are excluded to their own patches
- added fixes for samples/kdbus
- added fixes for selftests/kdbus

Sergei Zviagintsev (10):
kdbus: fix operator precedence issues in item macros
kdbus: use parentheses uniformly in KDBUS_ITEMS_FOREACH macro Documentation/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT
macro
Documentation/kdbus: use parentheses uniformly in KDBUS_ITEM_FOREACH
macro
selftests/kdbus: fix trivial style issues
selftests/kdbus: fix precedence issues in macros selftests/kdbus: use parentheses in iteration macros uniformly samples/kdbus: add whitespace
samples/kdbus: fix operator precedence issue in KDBUS_ITEM_NEXT macro samples/kdbus: use parentheses uniformly in KDBUS_FOREACH macro

Quoted 7 lines
 Documentation/kdbus/kdbus.item.xml         |  6 +++---
 ipc/kdbus/item.h                           | 10 ++++-----
 samples/kdbus/kdbus-api.h                  |  6 +++---
 tools/testing/selftests/kdbus/kdbus-enum.h |  1 +
 tools/testing/selftests/kdbus/kdbus-util.c |  2 +-
 tools/testing/selftests/kdbus/kdbus-util.h | 33 ++++++++++++++----------------
 6 files changed, 28 insertions(+), 30 deletions(-)

All now applied, thanks.

greg k-h