Tautological comparison in gcc 6.1.1
Michael Adam
obnox at samba.org
Tue Jul 12 22:15:53 UTC 2016
On 2016-07-12 at 17:32 +0200, Michael Adam wrote:
> 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.
Attached are patches that cover all definitions of DLIST_ADD_END
as above. We could add those with your authorship, amitay.
These are not the only problems encountered with gcc6.
Continuing now ...
-------------- next part --------------
From 66f209127bb1f6ad997237b7a340015a4949d69b Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 12 Jul 2016 23:37:22 +0200
Subject: [PATCH 1/3] util:dlinklist: avoid tautological compare errors in
DLIST_ADD_END
---
lib/util/dlinklist.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/util/dlinklist.h b/lib/util/dlinklist.h
index 8a1b84d..37cdcc3 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)
--
2.5.5
From 28b9bd06f4910faa40348c32c8c8592d62c4f8ca Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 12 Jul 2016 23:40:29 +0200
Subject: [PATCH 2/3] tevent: avoid tautological compare errors in
DLIST_ADD_END
---
lib/tevent/tevent_util.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/tevent/tevent_util.h b/lib/tevent/tevent_util.h
index e2cdbb8..fab8840 100644
--- a/lib/tevent/tevent_util.h
+++ b/lib/tevent/tevent_util.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)
--
2.5.5
From 77ccaa530569d1abb5b2fa674d4ef79b4e58c80f Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 12 Jul 2016 23:41:20 +0200
Subject: [PATCH 3/3] ldb: avoid tautological compare errors in DLIST_ADD_END
---
lib/ldb/include/dlinklist.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/ldb/include/dlinklist.h b/lib/ldb/include/dlinklist.h
index ef01aec..ebaa368 100644
--- a/lib/ldb/include/dlinklist.h
+++ b/lib/ldb/include/dlinklist.h
@@ -136,8 +136,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)
--
2.5.5
-------------- 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/20160713/890667a8/signature.sig>
More information about the samba-technical
mailing list