[Xcb] Invalid XIDs

[Nathaniel Smith]

On Sun, Jan 25, 2009 at 3:32 PM, Barton C Massey <bart at cs.pdx.edu> wrote:
> In message <961fa2b40901242247i4071c5b6va73d02e0523bd35d at mail.gmail.com> you wrote:
>> This latter part is arguably a bug in the X server or
>> something, but I don't see how it would ever matter, and
>> anyway, yes, count is always at least 1.
>
> Ow.  OK, we need code for that latter case.  Good catch.
> It's unlikely this is what happened to you, though, as the
> start_id was nowhere near 0 when you crashed.

Right.

> BTW, where did you find this information?  I can't see it
> anywhere.  What Xlib does is weird, as one might predict.

Xext/xcmisc.c and dix/resource.c in the xorg tree :-)

I would check with the server folks on this, if it's worth caring
about at all. It's not clear to me that it *is* worth caring about; if
you've managed to exhaust all the XIDs in your range (67 million?),
then you're both doing something ridiculous and are screwed anyway,
because even if the server wanted to help there wouldn't be any more
XIDs to give you. Or at least that's my understanding. Which is
presumably why the XC-MISC spec does not specify what should happen in
this case, and the current behavior is probably an ancient accident
(NB that xcmisc.c is still written in K&R style...).

(It's also unclear to me that there's anything anyone can do with an
error return from xcb_generate_id -- does anyone even check?  Is it
even guaranteed that (uint32_t)-1 indicates an error, rather than a
valid XID?)

> It looks like Xlib thinks count comes back 0 on failure, but
> then there's the martian line:
>
>  if (grep.count > 5)
>      dpy->resource_max += grep.count - 6;
>
> I have no idea what *that* is all about.  Why 5 or 6?

Strange; I would have thought X was too latency sensitive to use from
Mars. Well, learn something new every day.

> This code also brings up another issue.  While ubiquitous,
> technically XC_MISC is an extension, so we should be
> checking for its existence before trying to use it, I think.

I probably wouldn't bother until someone sent a bug report complaining
about it, but that's just me :-).

> BTW, a really cool implementation would fall back to
> XCMiscGetXIDList when XCMiscGetXIDRange failed, on the
> grounds that one might be able to reuse some old XIDs that
> way.  I haven't bothered, but maybe I should?

Surely GetXIDList and GetXIDRange inform you about the same XIDs, just
in different formats?

>> So I'm going to instrument the range resetting code
>> instead, and the next time the crash happens we'll know
>> for sure whether it was proceeded by a count==1 reply.
>
> Just stick an assertion for count > 1 in the new code.  If
> you ever hit it we will know what happened, and in the
> meantime you can be testing code we care about.

Hmm, good point. But unfortunately I've spent too long on this
already, and gdb instrumentation is quicker than getting a build
together (esp. since I already set up the instrumentation). Maybe next
week I'll have a chance.

Thanks,
-- Nathaniel