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

 
 

bug #43175: dubious sanity check in xinerama.c

Submitted by:  Jeff Abrahamson <jeff_abrahamson>
Submitted on:  Tue 09 Sep 2014 06:35:45 AM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

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

Fixed in 28d044c, thanks.

Jérémie Courrèges-Anglas <jca>
Project 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:

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

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>
Project 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>
Project 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 File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 28 Dec 2014 04:00:24 PM UTCjcaOpen/ClosedOpen=>Closed
    Wed 17 Sep 2014 08:13:56 AM UTCjcaStatusNeed Info=>Fixed
    Sun 14 Sep 2014 06:46:00 PM UTCjcaStatusNone=>Need Info
    Tue 09 Sep 2014 06:35:45 AM UTCjeff_abrahamsonCarbon-Copy-=>Added -unavailable-

    Back to the top


    Powered by Savane 3.1-cleanup1