build: work around gcc 6.0 warnings

Message ID 5709B77A.80706@redhat.com
State New
Headers show

Commit Message

Cole Robinson April 10, 2016, 2:16 a.m.
On 04/08/2016 12:27 PM, Andrea Bolognani wrote:
> On Mon, 2016-02-29 at 15:41 +0100, Martin Kletzander wrote:

>> On Wed, Feb 24, 2016 at 02:29:58PM -0700, Eric Blake wrote:

>>>  

>>> gcc 6.0 added an annoying warning:

>>>  

>>> fdstream.c: In function 'virFDStreamWrite':

>>> fdstream.c:390:29: error: logical 'or' of equal expressions [-Werror=logical-op]

>>>         if (errno == EAGAIN || errno == EWOULDBLOCK) {

>>>                             ^~

>>> fdstream.c: In function 'virFDStreamRead':

>>> fdstream.c:440:29: error: logical 'or' of equal expressions [-Werror=logical-op]

>>>         if (errno == EAGAIN || errno == EWOULDBLOCK) {

>>>                             ^~

>>>  

>>> This makes it impossible to build out-of-the-box on rawhide,

>>> and we aren't guaranteed that the gcc bug will be fixed in a

>>> timely manner:

>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602

>>>  

>>> So work around it by further complicating the logic to thwart the

>>> compiler.

>>>  

>>> Signed-off-by: Eric Blake <eblake@redhat.com>

>>> ---

>>>  

>>> This is a build-breaker fix for rawhide; but I'll wait for a day

>>> for any reasons why I should not push it during freeze.

>>>  

>> It looks like you're still talking this over and thinking about

>> approaches.  But could we push the fix for the time being so that the

>> release is nicely buildable and then work on making it nicer later?  I'd

>> vote ACK for that.

> 

> Update: the libvirt-fedora-rawhide build host on CentOS CI

> has been updated and it's now running gcc 6.0. The immediate

> result is that build jobs have started to fail (see [1] for

> an example).

> 

> Looking at the gcc bug, it doesn't look like there's been

> much movement in that area, so I think we should take some

> action on our side...

> 

> Eric, you mentioned proposing a v2 that would use #pragma

> instead of uglifying the current checks. Do you still

> intend to work on such alternative approach?

> 


I've attached a patch with the pragma push/pop approach Eric mentioned.
Unfortunately it doesn't work as is on RHEL6 vintage gcc so it's probably not
a realistic option.

> Failing that, I'm personally okay with pushing this.

> 


I agree

- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch

>From 5df55b50e143677d0b47b0e67ffd28184e4c0556 Mon Sep 17 00:00:00 2001
Message-Id: <5df55b50e143677d0b47b0e67ffd28184e4c0556.1460254498.git.crobinso@redhat.com>
From: Cole Robinson <crobinso@redhat.com>
Date: Sat, 9 Apr 2016 21:46:52 -0400
Subject: [PATCH] build: Avoid -Wlogical-op bug on gcc6

Use pragma push/pop to disable -Wlogical-op for the problematic
conditionals.

gcc bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602
https://www.redhat.com/archives/libvir-list/2016-February/msg00054.html
---
Unfortunately this doesn't work on RHEL6... gcc errors about #pragma
in a function call


 src/fdstream.c                  | 6 ++++++
 src/rpc/virnetsshsession.c      | 9 +++++++++
 src/security/security_selinux.c | 6 +++++-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/fdstream.c b/src/fdstream.c
index a85cf9d..0422ac5 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -387,7 +387,10 @@  static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes)
  retry:
     ret = write(fdst->fd, bytes, nbytes);
     if (ret < 0) {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wlogical-op"
         if (errno == EAGAIN || errno == EWOULDBLOCK) {
+#pragma GCC diagnostic pop
             ret = -2;
         } else if (errno == EINTR) {
             goto retry;
@@ -437,7 +440,10 @@  static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
  retry:
     ret = read(fdst->fd, bytes, nbytes);
     if (ret < 0) {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wlogical-op"
         if (errno == EAGAIN || errno == EWOULDBLOCK) {
+#pragma GCC diagnostic pop
             ret = -2;
         } else if (errno == EINTR) {
             goto retry;
diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c
index 406a831..a77571f 100644
--- a/src/rpc/virnetsshsession.c
+++ b/src/rpc/virnetsshsession.c
@@ -545,9 +545,12 @@  virNetSSHAuthenticateAgent(virNetSSHSessionPtr sess,
                                            agent_identity)))
             return 0; /* key accepted */
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wlogical-op"
         if (ret != LIBSSH2_ERROR_AUTHENTICATION_FAILED &&
             ret != LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED &&
             ret != LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED) {
+#pragma GCC diagnostic pop
             libssh2_session_last_error(sess->session, &errmsg, NULL, 0);
             virReportError(VIR_ERR_AUTH_FAILED,
                            _("failed to authenticate using SSH agent: %s"),
@@ -605,9 +608,12 @@  virNetSSHAuthenticatePrivkey(virNetSSHSessionPtr sess,
                                                    priv->password)) == 0)
         return 0; /* success */
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wlogical-op"
     if (priv->password ||
         ret == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED ||
         ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED) {
+#pragma GCC diagnostic pop
         libssh2_session_last_error(sess->session, &errmsg, NULL, 0);
         virReportError(VIR_ERR_AUTH_FAILED,
                        _("authentication with private key '%s' "
@@ -673,11 +679,14 @@  virNetSSHAuthenticatePrivkey(virNetSSHSessionPtr sess,
                          "has failed: %s"),
                        priv->filename, errmsg);
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wlogical-op"
         if (ret == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED ||
             ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED)
             return 1;
         else
             return -1;
+#pragma GCC diagnostic pop
     }
 
     return 0;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 26d95d1..b702fb3 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -911,8 +911,12 @@  virSecuritySELinuxSetFileconHelper(const char *path, char *tcon,
          * hopefully sets one of the necessary SELinux virt_use_{nfs,usb,pci}
          * boolean tunables to allow it ...
          */
-        if (setfilecon_errno != EOPNOTSUPP && setfilecon_errno != ENOTSUP &&
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wlogical-op"
+        if (setfilecon_errno != EOPNOTSUPP &&
+            setfilecon_errno != ENOTSUP &&
             setfilecon_errno != EROFS) {
+#pragma GCC diagnostic pop
             virReportSystemError(setfilecon_errno,
                                  _("unable to set security context '%s' on '%s'"),
                                  tcon, path);
-- 
2.7.3