Re: [RP] ratpoison-1.4.6 bug report

[Jérémie Courrèges-Anglas]

Jeff Abrahamson <address@hidden> writes:

> On 14 September 2014 20:51, Jérémie Courrèges-Anglas <address@hidden> wrote:
>
>> > On 8 September 2014 21:30, David Binderman <address@hidden> wrote:
>> >
>> >> Hello there,
>> >>
>> >> 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.
>>
>> Dubious, but how incorrect is it?
>
>
> 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.

Yup.

> I'll submit a patch soon-ish.

Don't bother, it's in.

Thank you both.
-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE