[libvirt] [PATCH 1/2] build: cleanup GCC < 4.6 -Wlogical-op workaround

[Martin Kletzander]

On Mon, Apr 11, 2016 at 12:07:26PM +0200, Pavel Hrdina wrote:
>On Mon, Apr 11, 2016 at 11:30:25AM +0200, Martin Kletzander wrote:
>> On Sun, Apr 10, 2016 at 06:37:58PM +0200, Pavel Hrdina wrote:
>> >Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>> >---
>> > m4/virt-compile-warnings.m4 |  2 +-
>> > src/internal.h              | 10 ++++++++++
>> > src/util/virbuffer.c        | 11 +++--------
>> > src/util/virstring.c        |  9 +--------
>> > src/util/virsysinfo.c       | 13 ++-----------
>> > 5 files changed, 17 insertions(+), 28 deletions(-)
>> >
>> >diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
>> >index 3dd0665..1b0a2cf 100644
>> >--- a/m4/virt-compile-warnings.m4
>> >+++ b/m4/virt-compile-warnings.m4
>> >@@ -236,7 +236,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
>> >
>> >     if test "$gl_cv_warn_c__Wlogical_op" = yes &&
>> >        test "$lv_cv_gcc_wlogical_op_broken" = yes; then
>> >-      AC_DEFINE_UNQUOTED([BROKEN_GCC_WLOGICALOP], 1,
>> >+      AC_DEFINE_UNQUOTED([BROKEN_GCC_WLOGICALOP_STRCHR], 1,
>> >        [Define to 1 if gcc -Wlogical-op reports false positives on strchr])
>> >     fi
>> > ])
>> >diff --git a/src/internal.h b/src/internal.h
>> >index 9ebaf3c..bc3126f 100644
>> >--- a/src/internal.h
>> >+++ b/src/internal.h
>> >@@ -253,6 +253,16 @@
>> > #  define VIR_WARNINGS_RESET
>> > # endif
>> >
>> >+/* Workaround bogus GCC < 4.6 that produces false -Wlogical-op warnings for
>> >+ * strchr(). Those old GCCs doesn't support push/pop. */
>>
>> s/doesn't/don't/
>
>Sigh, shame on me :)
>
>>
>> >+# if BROKEN_GCC_WLOGICALOP_STRCHR
>> >+#  define VIR_WARNINGS_NO_WLOGICALOP_STRCHR                \
>> >+    _Pragma ("GCC diagnostic ignored \"-Wlogical-op\"")
>> >+# else
>> >+#  define VIR_WARNINGS_NO_WLOGICALOP_STRCHR
>> >+# endif
>> >+
>> >+
>> > /*
>> >  * Use this when passing possibly-NULL strings to printf-a-likes.
>> >  */
>> >diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
>> >index 43cd1a7..d582e7d 100644
>> >--- a/src/util/virbuffer.c
>> >+++ b/src/util/virbuffer.c
>> >@@ -417,14 +417,9 @@ virBufferVasprintf(virBufferPtr buf, const char *format, va_list argptr)
>> >     buf->use += count;
>> > }
>> >
>> >-/* Work around spurious strchr() diagnostics given by -Wlogical-op
>> >- * for gcc < 4.6.  Doing it via a local pragma keeps the damage
>> >- * smaller than disabling it on the package level.  Unfortunately, the
>> >- * affected GCCs don't allow diagnostic push/pop which would have
>> >- * further reduced the impact. */
>>
>> To make the damage as small as possible, we could create a
>> src/util/virstrchr.c file in which this would be disabled, create
>> something like virStrChr() in it and then disable the use of strchr()
>> everywhere else in the code with syntax-check as we did with some other
>> string functions.  Was that discussed as well or should I send it as a
>> follow-up?
>
>That's actually a good idea and no, it wasn't discussed.  I'll push this series
>and yes send it as follow-up, its a good solution.
>

Just as an update, I worked on it and it's actually not.  It does spoil
so much of the codebase with super-ugly #include <virstrchr.h> (ugly
because it is in every freaking file) and it is only needed for places
where the output of strchr(haystack, needle) is used as a boolean (those
places that do (strchr(h, n) == h) are just stupid because they should
do h[0] == n or *h == n).  It got me so sick and tired that I stopped
caring so if anyone else wants to clean it up somehow, feel free to try.

>Thanks,
>
>Pavel
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160413/6548ea23/attachment-0001.sig>