[libvirt] Dealing with GCC 6.0

[Eric Blake]

On 02/01/2016 07:37 AM, Martin Kletzander wrote:
> So we recently started compiling with gcc 5 and there's a new thing now
> and we don't build on gcc 6.  I know it's just a development build, but
> it finds new things, like:
> 
> 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) {
>                             ^~
> 

POSIX allows EAGAIN and EWOULDBLOCK to be separate values; probably the
best fix for this would be some sort of macro or inline function that
does something like:

static int is_restartable(int err)
{
if (err == EAGAIN)
return true;
if (err == EWOULDBLOCK)
return true;
return false;
}

then using is_restartable(errno) rather than open-coding everywhere else
that we compare against EAGAIN and where POSIX says EWOULDBLOCK may be
returned.


> and
> 
> rpc/virnetsshsession.c: In function 'virNetSSHAuthenticateAgent':
> rpc/virnetsshsession.c:548:56: error: logical 'and' of equal expressions
> [-Werror=logical-op]
>         if (ret != LIBSSH2_ERROR_AUTHENTICATION_FAILED &&
>                                                        ^~
> rpc/virnetsshsession.c: In function 'virNetSSHAuthenticatePrivkey':
> rpc/virnetsshsession.c:676:57: error: logical 'or' of equal expressions
> [-Werror=logical-op]
>         if (ret == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED ||
>                                                         ^~
> 

More context:

  if (ret != LIBSSH2_ERROR_AUTHENTICATION_FAILED &&
            ret != LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED &&
            ret != LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED) {

        if (ret == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED ||
            ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED)

so depending on the libssh2 build, PUBLICKEY_UNRECOGNIZED and
AUTHENTICATION_FAILED must be the same value.  Again, something where we
could write an inline wrapper to split up the checks to bypass the
-Wlogical-op stupidity without making the code too unreadable.

> The first one is prety easy to fix, that's the only place where we use
> EWOULDBLOCK, we are using only EAGAIN everywhere else.

Dropping the 'errno == EWOULDBLOCK' branch would be a bug.

>  However, the
> second one is not that easy to change for me as I'm not a libssh expert
> and moreover, I would like to know other's opinions on how to tackle
> that.  We can disable the logical-op warning, but it might show
> something that makes sense (really?).  Or we can do bunch of very very
> ugly "#if"s.
> 
> Anyone care to share an idea?

I've filed a gcc bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602

If it's only rawhide that is failing, the next question is whether
rawhide is using unreleased gcc on purpose to find problems like this
before gcc releases, or whether anyone using stock gcc 6.0 tarball will
face the same problem.  Disabling -Wlogical-op feels like too heavy of a
hammer, especially if my idea to break the checks up into multiple 'if's
in an inline wrapper function works.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160201/693d36c1/attachment-0001.sig>