Re: ratpoison-1.4.6 bug report

On 14 September 2014 20:51, Jérémie Courrèges-Anglas <[email protected]> wrote:

> > On 8 September 2014 21:30, David Binderman <[email protected]> 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.

I'll submit a patch soon-ish.

_______________________________________________
Ratpoison-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/ratpoison-devel