lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANq1E4S4vy+BTnj=ifd0NSE3DoSc=aS998Lqt-GCWe5TAXWQyw@mail.gmail.com>
Date:	Thu, 4 Jun 2015 13:29:22 +0200
From:	David Herrmann <dh.herrmann@...il.com>
To:	Sergei Zviagintsev <sergei@...v.net>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Daniel Mack <daniel@...que.org>,
	David Herrmann <dh.herrmann@...glemail.com>,
	Djalal Harouni <tixxdz@...ndz.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kdbus: use parentheses in KDBUS_ITEM_FOREACH macro uniformly

Hi

On Thu, Jun 4, 2015 at 12:39 PM, Sergei Zviagintsev <sergei@...v.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@...v.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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ