[Xcb] [RFC] New project: util-errors for translating numbers into request/error/event names

[Uli Schlachter]

Hi,

On 27.03.2015 20:58, Ran Benita wrote:
> On Wed, Mar 25, 2015 at 07:02:25PM +0100, Uli Schlachter wrote:
[...]
>> inspired by an issue that showed up in Qt, I started working on an
>> util-errors library that does something similar to Xlib's XErrorDB. By
>> now this library is progressing nicely and I'm interested in feedback
>> from others.
>>
>> Currently it can be found on GitHub:
>>
>> https://github.com/psychon/util-errors
>
> That's a useful addition. Making errors better is a good cause :)
> I've skimmed through the code, haven't tried it yet, so can't comment
> on the functionality for now.
>
> make check fails for me:
>      For event 89: Expected ScreenChangeNotify, got RawTouchUpdate
>      For event 90: Expected Notify, got RawTouchEnd

Yeah, I test some RandR things and XKB "messes things up". I have an idea how to 
improve the code to handle this and I should definitely fix XKB, but I don't 
have the time to work on this code this week.

> Btw: if X is not available for the test, I think it's better to SKIP it
> (exit with status 77) instead of FAILing it.

Thanks, I was too lazy to look up which exit code this was. :-)

> I see that you generate extensions.c during build. Have you considered
> commiting it to git instead? That way all versions are the same, and do
> not depend on the builder's environment. If there's a change you can
> release a new version. (I'm not arguing for that - just a
> consideration).

Generated code doesn't belong into git. :-P

However, I could think about having "make dist" include it into the release. I'm 
not convinced if that is a good idea, since I expected this code to change 
rarely (and thus rarely have new releases), but xcb-proto might "often" be 
enhanced with new extensions... On the other hand, which distro will rebuild the 
library for newer xcb-proto versions?

> Is it not possible to make it entirely static? (i.e. no memory
> allocations / requests at all). (Are first_event/first_error dynamic?)

Nope, first_event and first_error are dynamically assigned by the X11 server.

> In register_extension(), why return 0 if calloc fails?

That's a good question. I will look into the error handling here again. (And I 
guess it should not ignore any error at all.

> Finally, I think a library whose purpose is informative errors should
> itself return informative errors :) Maybe <errno.h> codes?

You mean returning -ENOMEM when calloc() fails?
The only other possible error would be "some X11 request failed" and I don't 
know which error code to use for this. I'll think about it.

[...]
>> Also, right now xcb_errors_get_name_for_minor_code() returns NULL if the
>> major code belongs to the core protocol. I guess that for arguments in
>> the range 0 to 127 it should behave like *_for_major_code() instead to simplify
>> usage.
>>
>> For the output, it might make sense to include the extension name in the
>> result. For example "RandR QueryVersion" is more helpful than just
>> "QueryVersion".
>
> Why not xcb_errors_get_name_for_extension()?

That would end up being xcb_errors_get_extension_name_for_event() and 
*_for_error(), right? Although the *_for_error() function would be useless, 
since error contain a major code. (There is already *_for_major_code() for requests)

[...]

Thanks for taking a look,
Uli