[libvirt] Dealing with GCC 6.0

[Martin Kletzander]

On Mon, Feb 01, 2016 at 03:14:04PM -0700, Eric Blake wrote:
>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.
>

That means we should use EWOULDBLOCK in other places where we check for
EAGAIN only as well, right?  Or only where the read/write can be done
through sockets.

>>  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.
>

I should've dug more into this, thanks for doing it instead of me.
Although I wouldn't describe the behaviour so nicely in the bug as you
did.  Let's hope this will be fixed in GCC as that is, as I said, only a
rawhide devel build, there is no release yet, at least not to my
knowledge.

Have a nice day,
Martin
-------------- 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/20160202/0e338405/attachment-0001.sig>