Tautological comparison in gcc 6.1.1

[Michael Adam]

Hi Amitay,

I just stumbled across the same issue.
Thanks for already having proposed a solution!

On 2016-07-06 at 12:26 +1000, Amitay Isaacs wrote:
> Hi,
> 
> In gcc 6.1.1 (from fedora 24), there is a new warning for tautological
> comparisons.  It issues a warning for DLIST_ADD_END(), so I cannot use
> --picky-developer any more.
> 
> Here is the preprocessor output for statement DLIST_ADD_END(list, p)
> 
>  do { if (!(list)) { do { if (!(list)) { (p)->prev = (list) = (p);
> (p)->next =
> ((void *)0)
> ; } else { (p)->prev = (list)->prev; (list)->prev = (p); (p)->next =
> (list); (list) = (p); } } while (0); } else { do { if (!(list) ||
> !((list)->prev)) { do { if (!(list)) { (p)->prev = (list) = (p); (p)->next
> =
> ((void *)0)
> ; } else { (p)->prev = (list)->prev; (list)->prev = (p); (p)->next =
> (list); (list) = (p); } } while (0); } else { (p)->prev = ((list)->prev);
> (p)->next = ((list)->prev)->next; ((list)->prev)->next = (p); if
> ((p)->next) (p)->next->prev = (p); if ((list)->prev == ((list)->prev))
> (list)->prev = (p); }} while (0); } } while (0);
> 
> In the last else, which is expansion for DLIST_ADD_AFTER(), there is a
> tautological comparison.
> 
> if ((list)->prev == ((list)->prev) ...
> 
> This comes from:
> 
> #define DLIST_ADD_AFTER(list, p, el) \
> do { \
>         if (!(list) || !(el)) { \
>         DLIST_ADD(list, p); \
>     } else { \
>         (p)->prev = (el);   \
>         (p)->next = (el)->next;        \
>         (el)->next = (p);        \
>         if ((p)->next) (p)->next->prev = (p);    \
>         if ((list)->prev == (el)) (list)->prev = (p); \            <-----
> culprit
>     }\
> } while (0)
> 
> One solution would be embed DLIST_ADD_AFTER() code in DLIST_ADD_END() and
> drop the check in the last statement as follows:
> 
> diff --git a/lib/util/dlinklist.h b/lib/util/dlinklist.h
> index 8a1b84d..e969f17 100644
> --- a/lib/util/dlinklist.h
> +++ b/lib/util/dlinklist.h
> @@ -132,8 +132,14 @@ do { \
>  do { \
>         if (!(list)) { \
>                 DLIST_ADD(list, p); \
> +       } else if (!((list)->prev)) { \
> +               DLIST_ADD(list, p); \
>         } else { \
> -               DLIST_ADD_AFTER(list, p, (list)->prev); \
> +               (p)->prev = (list)->prev; \
> +               (p)->next = (list)->prev->next; \
> +               (list)->prev->next = (p); \
> +               if ((p)->next) (p)->next->prev = (p); \
> +               (list)->prev = (p); \
>         } \
>  } while (0)
> 
> 
> Is there a better solution?

The code duplication instead of re-use lacks elegance,
but it is the best solition I can think of, currently
(without disabling the Werror flag on all files using
the dlist code. Btw, we have additional copies in tevent
and ldb that need the same fixing.

Not sure whether the implementers of -Wtautological-comparison
have conciously created warnings/errors for advanced use
of macros... :-}

Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160712/cb139c4f/signature.sig>