2015-06-09 21:00:30

by Sergei Zviagintsev

Subject: [PATCH v2 00/10] 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(-)

--
1.8.3.1

2015-06-09 21:00:42

by Sergei Zviagintsev

Subject: [PATCH v2 01/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.

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
ipc/kdbus/item.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipc/kdbus/item.h b/ipc/kdbus/item.h
index eeefd8beac3b..32909e2e7954 100644
--- a/ipc/kdbus/item.h
+++ b/ipc/kdbus/item.h
@@ -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

2015-06-09 21:00:58

by Sergei Zviagintsev

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

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
ipc/kdbus/item.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipc/kdbus/item.h b/ipc/kdbus/item.h
index 32909e2e7954..bca63b4e6e80 100644
--- a/ipc/kdbus/item.h
+++ b/ipc/kdbus/item.h
@@ -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

2015-06-09 21:01:02

by Sergei Zviagintsev

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

`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.

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
Documentation/kdbus/kdbus.item.xml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/kdbus/kdbus.item.xml b/Documentation/kdbus/kdbus.item.xml
index 09f8b903116f..b0eeeef995af 100644
--- a/Documentation/kdbus/kdbus.item.xml
+++ b/Documentation/kdbus/kdbus.item.xml
@@ -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

2015-06-09 21:00:53

by Sergei Zviagintsev

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

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
Documentation/kdbus/kdbus.item.xml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/kdbus/kdbus.item.xml b/Documentation/kdbus/kdbus.item.xml
index b0eeeef995af..ee09dfa443b8 100644
--- a/Documentation/kdbus/kdbus.item.xml
+++ b/Documentation/kdbus/kdbus.item.xml
@@ -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>
--
1.8.3.1

2015-06-09 21:01:11

by Sergei Zviagintsev

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
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(-)

diff --git a/tools/testing/selftests/kdbus/kdbus-enum.h b/tools/testing/selftests/kdbus/kdbus-enum.h
index a67cec3512a7..ed28cca26906 100644
--- a/tools/testing/selftests/kdbus/kdbus-enum.h
+++ b/tools/testing/selftests/kdbus/kdbus-enum.h
@@ -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);
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index 4b376ecfdbed..1e267d585a14 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -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;

diff --git a/tools/testing/selftests/kdbus/kdbus-util.h b/tools/testing/selftests/kdbus/kdbus-util.h
index 50ff07140bdd..b53b03f0565c 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.h
+++ b/tools/testing/selftests/kdbus/kdbus-util.h
@@ -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

2015-06-09 21:01:23

by Sergei Zviagintsev

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

`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.

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
tools/testing/selftests/kdbus/kdbus-util.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kdbus/kdbus-util.h b/tools/testing/selftests/kdbus/kdbus-util.h
index b53b03f0565c..df5721ee8f54 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.h
+++ b/tools/testing/selftests/kdbus/kdbus-util.h
@@ -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

2015-06-09 21:01:32

by Sergei Zviagintsev

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

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
tools/testing/selftests/kdbus/kdbus-util.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kdbus/kdbus-util.h b/tools/testing/selftests/kdbus/kdbus-util.h
index df5721ee8f54..d1a0f1b4d0eb 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.h
+++ b/tools/testing/selftests/kdbus/kdbus-util.h
@@ -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

2015-06-09 21:02:05

by Sergei Zviagintsev

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
samples/kdbus/kdbus-api.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/kdbus/kdbus-api.h b/samples/kdbus/kdbus-api.h
index 5ed5907c5cb4..2de4d6a8c51e 100644
--- a/samples/kdbus/kdbus-api.h
+++ b/samples/kdbus/kdbus-api.h
@@ -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

2015-06-09 21:01:45

by Sergei Zviagintsev

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

`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.

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
samples/kdbus/kdbus-api.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/kdbus/kdbus-api.h b/samples/kdbus/kdbus-api.h
index 2de4d6a8c51e..fab873b89d97 100644
--- a/samples/kdbus/kdbus-api.h
+++ b/samples/kdbus/kdbus-api.h
@@ -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

2015-06-09 21:01:56

by Sergei Zviagintsev

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

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
samples/kdbus/kdbus-api.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/samples/kdbus/kdbus-api.h b/samples/kdbus/kdbus-api.h
index fab873b89d97..7f3abae18396 100644
--- a/samples/kdbus/kdbus-api.h
+++ b/samples/kdbus/kdbus-api.h
@@ -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

2015-06-10 13:44:48

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 10:59 PM, Sergei Zviagintsev <[email protected]> wrote:
> `_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.
>
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> ipc/kdbus/item.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/ipc/kdbus/item.h b/ipc/kdbus/item.h
> index eeefd8beac3b..32909e2e7954 100644
> --- a/ipc/kdbus/item.h
> +++ b/ipc/kdbus/item.h
> @@ -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
>

2015-06-10 13:46:26

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> Enclose all arguments into parentheses to stay consistent across the
> whole macro.
>
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> ipc/kdbus/item.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/ipc/kdbus/item.h b/ipc/kdbus/item.h
> index 32909e2e7954..bca63b4e6e80 100644
> --- a/ipc/kdbus/item.h
> +++ b/ipc/kdbus/item.h
> @@ -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
>

2015-06-10 14:03:25

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> `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.
>
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> Documentation/kdbus/kdbus.item.xml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/Documentation/kdbus/kdbus.item.xml b/Documentation/kdbus/kdbus.item.xml
> index 09f8b903116f..b0eeeef995af 100644
> --- a/Documentation/kdbus/kdbus.item.xml
> +++ b/Documentation/kdbus/kdbus.item.xml
> @@ -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
>

2015-06-10 14:04:01

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> Enclose all arguments into parentheses to stay consistent across the
> whole macro.
>
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> Documentation/kdbus/kdbus.item.xml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/kdbus/kdbus.item.xml b/Documentation/kdbus/kdbus.item.xml
> index b0eeeef995af..ee09dfa443b8 100644
> --- a/Documentation/kdbus/kdbus.item.xml
> +++ b/Documentation/kdbus/kdbus.item.xml
> @@ -72,10 +72,10 @@
> (typeof(item))((uint8_t *)(item) + KDBUS_ALIGN8((item)->size))

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> #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
>

2015-06-10 14:06:11

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> 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(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/tools/testing/selftests/kdbus/kdbus-enum.h b/tools/testing/selftests/kdbus/kdbus-enum.h
> index a67cec3512a7..ed28cca26906 100644
> --- a/tools/testing/selftests/kdbus/kdbus-enum.h
> +++ b/tools/testing/selftests/kdbus/kdbus-enum.h
> @@ -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);
> diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
> index 4b376ecfdbed..1e267d585a14 100644
> --- a/tools/testing/selftests/kdbus/kdbus-util.c
> +++ b/tools/testing/selftests/kdbus/kdbus-util.c
> @@ -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;
>
> diff --git a/tools/testing/selftests/kdbus/kdbus-util.h b/tools/testing/selftests/kdbus/kdbus-util.h
> index 50ff07140bdd..b53b03f0565c 100644
> --- a/tools/testing/selftests/kdbus/kdbus-util.h
> +++ b/tools/testing/selftests/kdbus/kdbus-util.h
> @@ -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
>

2015-06-10 14:07:05

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> `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.
>
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> tools/testing/selftests/kdbus/kdbus-util.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/tools/testing/selftests/kdbus/kdbus-util.h b/tools/testing/selftests/kdbus/kdbus-util.h
> index b53b03f0565c..df5721ee8f54 100644
> --- a/tools/testing/selftests/kdbus/kdbus-util.h
> +++ b/tools/testing/selftests/kdbus/kdbus-util.h
> @@ -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
>

2015-06-10 14:08:32

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> Enclose all arguments into parentheses in KDBUS_ITEM_FOREACH and
> KDBUS_FOREACH macros to stay consistent across the whole macro.
>
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> tools/testing/selftests/kdbus/kdbus-util.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/tools/testing/selftests/kdbus/kdbus-util.h b/tools/testing/selftests/kdbus/kdbus-util.h
> index df5721ee8f54..d1a0f1b4d0eb 100644
> --- a/tools/testing/selftests/kdbus/kdbus-util.h
> +++ b/tools/testing/selftests/kdbus/kdbus-util.h
> @@ -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
>

2015-06-10 14:09:37

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> samples/kdbus/kdbus-api.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/samples/kdbus/kdbus-api.h b/samples/kdbus/kdbus-api.h
> index 5ed5907c5cb4..2de4d6a8c51e 100644
> --- a/samples/kdbus/kdbus-api.h
> +++ b/samples/kdbus/kdbus-api.h
> @@ -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
>

2015-06-10 14:09:56

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> `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.
>
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> samples/kdbus/kdbus-api.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/samples/kdbus/kdbus-api.h b/samples/kdbus/kdbus-api.h
> index 2de4d6a8c51e..fab873b89d97 100644
> --- a/samples/kdbus/kdbus-api.h
> +++ b/samples/kdbus/kdbus-api.h
> @@ -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
>

2015-06-10 14:11:32

by David Herrmann

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

Hi

On Tue, Jun 9, 2015 at 11:00 PM, Sergei Zviagintsev <[email protected]> wrote:
> Enclose all arguments into parentheses to stay consistent across the
> whole macro.
>
> Signed-off-by: Sergei Zviagintsev <[email protected]>
> ---
> samples/kdbus/kdbus-api.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: David Herrmann <[email protected]>

Thanks
David

> diff --git a/samples/kdbus/kdbus-api.h b/samples/kdbus/kdbus-api.h
> index fab873b89d97..7f3abae18396 100644
> --- a/samples/kdbus/kdbus-api.h
> +++ b/samples/kdbus/kdbus-api.h
> @@ -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
>

2015-06-10 14:17:23

by David Herrmann

Subject: Re: [PATCH v2 00/10] kdbus: macros fixes

Hi

On Tue, Jun 9, 2015 at 10:59 PM, Sergei Zviagintsev <[email protected]> wrote:
> 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

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

2015-06-10 14:56:18

by Sergei Zviagintsev

Subject: Re: [PATCH v2 00/10] kdbus: macros fixes

Hi

On Wed, Jun 10, 2015 at 04:13:49PM +0200, David Herrmann wrote:
> 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.

2015-06-19 05:05:11

by Greg Kroah-Hartman

Subject: Re: [PATCH v2 00/10] kdbus: macros fixes

On Tue, Jun 09, 2015 at 11:59:58PM +0300, Sergei Zviagintsev wrote:
> 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(-)

All now applied, thanks.

greg k-h