bugratpoison - Bugs: bug #43175, dubious sanity check in xinerama.c

 
 

bug #43175: dubious sanity check in xinerama.c

Submitter:  Jeff Abrahamson <jeff_abrahamson>
Submitted:  Tue 09 Sep 2014 06:35:45 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 17 Sep 2014 08:13:56 AM UTC, comment #5: 

Fixed in 28d044c, thanks.

Jérémie Courrèges-Anglas <jca>
Group administrator
Wed 17 Sep 2014 07:42:20 AM UTC, comment #4: 

I'm not clear that it's incorrect, but I'll readily agree that the more I look at it, the less I like it. I was initially tempted to fix it just by adding the parentheses that don't change meaning but do make it impossible for a human to mis-parse.

But I don't think that would be the Right Thing To Do. From the XineramaQueryVersion man page:


   If  the  Xinerama  library is compatible with the version returned by the server,
   it returns nonzero. If the server does not support the XINERAMA extension, or if there
   was an error during communications with the server, or if the server and library
   protocol  versions  are incompatible, it returns zero.


Success is defined in /usr/include/X11/X.h with value zero. But here zero is the error value, so I'm not keen on using the symbolic constant Success, since it is both at odds with the manpage and with the sense of what is happening. If XineramaQueryVersion() returns non-zero, it means that we correctly found the xinerama major and minor version numbers. If it returns 0 (Success), then it means we've failed and should drop out of xinerama_init() without setting rp_have_xinerama.

I think, therefore, that the correct line should be


   if (!XineramaQueryVersion(dpy, &major, &minor)) {
       return;
   }


which, admittedly (and thankfully), must have the same behavior.

Jeff Abrahamson <jeff_abrahamson>
Sun 14 Sep 2014 07:16:51 PM UTC, comment #3: 

It was not an advice but an admittedly terse call for more input about this issue. ;-)


Jérémie Courrèges-Anglas <jca>
Group administrator
Sun 14 Sep 2014 07:05:29 PM UTC, comment #2: 


>A compiler warning is not a bug.


I am grateful for your advice.

Suggest rework code to ensure that a boolean expression
isn't compared with the value of Success.

Maybe

if (XineramaQueryVersion(dpy, &major, &minor) != Success) {
return;
}

was intended ?

David Binderman <dcb314>
Sun 14 Sep 2014 06:46:00 PM UTC, comment #1: 

A compiler warning is not a bug.

Jérémie Courrèges-Anglas <jca>
Group administrator
Tue 09 Sep 2014 06:35:45 AM UTC, original submission:  

David Binderman <dcb314@hotmail.com> reports:

This bug is also in trunk. I just checked.

xinerama.c:50:56: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]

        if (!XineramaQueryVersion(dpy, &major, &minor) != Success) {
                return;
        }

Clearly some dubious sanity checking code. Suggest code rework.

Jeff Abrahamson <jeff_abrahamson>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dcb314 (Posted a comment)
  • -email is unavailable- added by jca (Posted a comment)
  • -email is unavailable- added by jeff_abrahamson (Submitted the item)
  • -email is unavailable- added by jeff_abrahamson
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-12-28 jca Open/ClosedOpen Closed
    2014-09-17 jca StatusNeed Info Fixed
    2014-09-14 jca StatusNone Need Info
    2014-09-09 jeff_abrahamson Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code