* [PATCH] kdbus: use parentheses in KDBUS_ITEM_FOREACH macro uniformly
@ 2015-06-04 10:39 Sergei Zviagintsev
2015-06-04 11:29 ` David Herrmann
0 siblings, 1 reply; 3+ messages in thread
From: Sergei Zviagintsev @ 2015-06-04 10:39 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: linux-kernel, Sergei Zviagintsev
_i is used as loop cursor and must be a proper lvalue, whereas two other
arguments can be complex expressions. Stay uniform across the macro and
enclose _is and _s with parentheses in all cases, but keep _i without
them (any valid lvalue will not break cast to (u8 *) due to precedence
rules). Update documentation example.
List macroses (e.g. list_for_each) were taken as an example for this
change.
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
Documentation/kdbus/kdbus.item.xml | 4 ++--
ipc/kdbus/item.h | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/kdbus/kdbus.item.xml b/Documentation/kdbus/kdbus.item.xml
index b0eeeef995af..fb8b6c151a0d 100644
--- a/Documentation/kdbus/kdbus.item.xml
+++ b/Documentation/kdbus/kdbus.item.xml
@@ -73,8 +73,8 @@
#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) + (head)->size) && \
+ ((uint8_t *)item >= (uint8_t *)(head)); \
item = KDBUS_ITEM_NEXT(item))
]]></programlisting>
</refsect2>
diff --git a/ipc/kdbus/item.h b/ipc/kdbus/item.h
index 03612368b3bb..592b68a7c2db 100644
--- a/ipc/kdbus/item.h
+++ b/ipc/kdbus/item.h
@@ -28,9 +28,9 @@
#define KDBUS_ITEM_PAYLOAD_SIZE(_i) ((_i)->size - KDBUS_ITEM_HEADER_SIZE)
#define KDBUS_ITEMS_FOREACH(_i, _is, _s) \
- for (_i = _is; \
- ((u8 *)(_i) < (u8 *)(_is) + (_s)) && \
- ((u8 *)(_i) >= (u8 *)(_is)); \
+ for (_i = (_is); \
+ ((u8 *)_i < (u8 *)(_is) + (_s)) && \
+ ((u8 *)_i >= (u8 *)(_is)); \
_i = KDBUS_ITEM_NEXT(_i))
#define KDBUS_ITEM_VALID(_i, _is, _s) \
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] kdbus: use parentheses in KDBUS_ITEM_FOREACH macro uniformly
2015-06-04 10:39 [PATCH] kdbus: use parentheses in KDBUS_ITEM_FOREACH macro uniformly Sergei Zviagintsev
@ 2015-06-04 11:29 ` David Herrmann
2015-06-04 11:47 ` Sergei Zviagintsev
0 siblings, 1 reply; 3+ messages in thread
From: David Herrmann @ 2015-06-04 11:29 UTC (permalink / raw)
To: Sergei Zviagintsev
Cc: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
linux-kernel
Hi
On Thu, Jun 4, 2015 at 12:39 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> _i is used as loop cursor and must be a proper lvalue, whereas two other
> arguments can be complex expressions. Stay uniform across the macro and
> enclose _is and _s with parentheses in all cases, but keep _i without
> them (any valid lvalue will not break cast to (u8 *) due to precedence
> rules). Update documentation example.
>
> List macroses (e.g. list_for_each) were taken as an example for this
> change.
I'm not a big fan of this patch. Treating arguments differently just
makes the macros harder to read, imo. Why not keep the parantheses
around all arguments? It doesn't hurt and keeps the style consistent.
Besides, lvalues might consist of more complex expressions, so I'm not
sure this is even the right way to go.
Thanks
David
> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
> Documentation/kdbus/kdbus.item.xml | 4 ++--
> ipc/kdbus/item.h | 6 +++---
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/kdbus/kdbus.item.xml b/Documentation/kdbus/kdbus.item.xml
> index b0eeeef995af..fb8b6c151a0d 100644
> --- a/Documentation/kdbus/kdbus.item.xml
> +++ b/Documentation/kdbus/kdbus.item.xml
> @@ -73,8 +73,8 @@
>
> #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) + (head)->size) && \
> + ((uint8_t *)item >= (uint8_t *)(head)); \
> item = KDBUS_ITEM_NEXT(item))
> ]]></programlisting>
> </refsect2>
> diff --git a/ipc/kdbus/item.h b/ipc/kdbus/item.h
> index 03612368b3bb..592b68a7c2db 100644
> --- a/ipc/kdbus/item.h
> +++ b/ipc/kdbus/item.h
> @@ -28,9 +28,9 @@
> #define KDBUS_ITEM_PAYLOAD_SIZE(_i) ((_i)->size - KDBUS_ITEM_HEADER_SIZE)
>
> #define KDBUS_ITEMS_FOREACH(_i, _is, _s) \
> - for (_i = _is; \
> - ((u8 *)(_i) < (u8 *)(_is) + (_s)) && \
> - ((u8 *)(_i) >= (u8 *)(_is)); \
> + for (_i = (_is); \
> + ((u8 *)_i < (u8 *)(_is) + (_s)) && \
> + ((u8 *)_i >= (u8 *)(_is)); \
> _i = KDBUS_ITEM_NEXT(_i))
>
> #define KDBUS_ITEM_VALID(_i, _is, _s) \
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] kdbus: use parentheses in KDBUS_ITEM_FOREACH macro uniformly
2015-06-04 11:29 ` David Herrmann
@ 2015-06-04 11:47 ` Sergei Zviagintsev
0 siblings, 0 replies; 3+ messages in thread
From: Sergei Zviagintsev @ 2015-06-04 11:47 UTC (permalink / raw)
To: David Herrmann
Cc: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
linux-kernel
Hi David,
On Thu, Jun 04, 2015 at 01:29:22PM +0200, David Herrmann wrote:
> Hi
>
> On Thu, Jun 4, 2015 at 12:39 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> > _i is used as loop cursor and must be a proper lvalue, whereas two other
> > arguments can be complex expressions. Stay uniform across the macro and
> > enclose _is and _s with parentheses in all cases, but keep _i without
> > them (any valid lvalue will not break cast to (u8 *) due to precedence
> > rules). Update documentation example.
> >
> > List macroses (e.g. list_for_each) were taken as an example for this
> > change.
>
> I'm not a big fan of this patch. Treating arguments differently just
> makes the macros harder to read, imo. Why not keep the parantheses
> around all arguments? It doesn't hurt and keeps the style consistent.
I agree, the point is just to keep things consistent. I'll send v2.
> Besides, lvalues might consist of more complex expressions, so I'm not
> sure this is even the right way to go.
>
> Thanks
> David
>
> > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> > ---
> > Documentation/kdbus/kdbus.item.xml | 4 ++--
> > ipc/kdbus/item.h | 6 +++---
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/kdbus/kdbus.item.xml b/Documentation/kdbus/kdbus.item.xml
> > index b0eeeef995af..fb8b6c151a0d 100644
> > --- a/Documentation/kdbus/kdbus.item.xml
> > +++ b/Documentation/kdbus/kdbus.item.xml
> > @@ -73,8 +73,8 @@
> >
> > #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) + (head)->size) && \
> > + ((uint8_t *)item >= (uint8_t *)(head)); \
> > item = KDBUS_ITEM_NEXT(item))
> > ]]></programlisting>
> > </refsect2>
> > diff --git a/ipc/kdbus/item.h b/ipc/kdbus/item.h
> > index 03612368b3bb..592b68a7c2db 100644
> > --- a/ipc/kdbus/item.h
> > +++ b/ipc/kdbus/item.h
> > @@ -28,9 +28,9 @@
> > #define KDBUS_ITEM_PAYLOAD_SIZE(_i) ((_i)->size - KDBUS_ITEM_HEADER_SIZE)
> >
> > #define KDBUS_ITEMS_FOREACH(_i, _is, _s) \
> > - for (_i = _is; \
> > - ((u8 *)(_i) < (u8 *)(_is) + (_s)) && \
> > - ((u8 *)(_i) >= (u8 *)(_is)); \
> > + for (_i = (_is); \
> > + ((u8 *)_i < (u8 *)(_is) + (_s)) && \
> > + ((u8 *)_i >= (u8 *)(_is)); \
> > _i = KDBUS_ITEM_NEXT(_i))
> >
> > #define KDBUS_ITEM_VALID(_i, _is, _s) \
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-06-04 11:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-04 10:39 [PATCH] kdbus: use parentheses in KDBUS_ITEM_FOREACH macro uniformly Sergei Zviagintsev
2015-06-04 11:29 ` David Herrmann
2015-06-04 11:47 ` Sergei Zviagintsev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox