[Xcb] XCB error handling; my penultimate proposal
Barton C Massey
bart at cs.pdx.edu
Mon Dec 19 12:59:45 PST 2005
So Jamey's new proposal
5. Get rid of the error pointer in *Reply(). Make new
versions of the API functions called *Error() that
take the error pointer instead.
(As for the XSync() thing, I suspect that it makes sense to
provide XSync() and XSyncErr(), where the *Err() is the one
returned by the noop itself?)
The two reasons I prefer 3 over 5 are: (a) while I agree
that if we're going to change the API we should do it sooner
rather than later, I don't think this change in semantics is
a sufficient reason to change it, and (b) you're now
supplying a pointer to the library that is set aside and
filled in "later".
Other libraries tend not to do that, because it is
error-prone and difficult to work with. Note, for example,
that if the storage to receive the error is allocated by the
caller, this storage must remain valid until the cookie is
forced, which means that a local can't be used if the cookie
is not forced in the dynamic scope of the request.
Conversely, if it's allocated by the library, it must then
be disposed of by the caller, which is going to lead to
memory leaks.
The imputed advantage of 5 over 3 is that folks using 3
screw up and have the app dump core. But I can live with
this by comparison---it's easy to find and fix.
Right now, I guess I'd vote
best = 3 > 5 > 4 > 2 > 1 > 0 = worst
where 0 is to leave things as they are.
Let me say a couple more words about why I'm excited about
3/5. My envisioned error handling for most X apps is like
this:
(a) Most of the time, if I get an error from the X server
in a GUI app, I just want to gracefully shut down.
This is quite convenient to do from the event handler.
(b) Occasionally, I'll make a request that I know might
fail, and want to handle the expected failure or
shutdown otherwise. For example, I might choose to
try to read a property off the display knowing it
might not be there. In this case, it is quite
convenient to be able to ask to see any error that
is returned for the request.
Right now, simple Xlib apps either do (a) or just ignore
errors altogether. Scanning over xcb-demo:
xdpyinfo.c: Only looks to see whether a couple of
requests with replies failed, and ignores any failure
for them. Never processes events. :-)
xcbrandr.c: Used the err argument to *Reply() in an
interesting way in only one place. Lots of
potentially buggy error handling that I excised. Note
that this code also does not process events, so those
schemes are out.
dpms.c: Does no error checking or event processing at all.
hypnomoire.c: Does its error processing in the event
loop---prints a warning and ignores the error. Passes
0 to the error argument for its one request that gets
a reply.
main.c [xcb-test]: Shares code with hypnomoire.c, and
works the same way.
rendertest.c: Assumes the server has the render
extension (!). Does no checking of replies, passes 0
as the err field. Does no event processing.
tests/julia.c: Checks replies for null, passes 0 as the
err field. Otherwise, ignores errors.
tests/lissajoux.c: Checks its one reply for null, which
makes sense since it's checking whether it can use
MIT-SHM. Passes NULL as the err field. Otherwise,
ignores errors.
What do toolkits etc do now?
Bart
In message <20051219182726.GE7478 at id.minilop.net> you wrote:
>
> --===============1146270724==
> Content-Type: multipart/signed; micalg=pgp-sha1;
> protocol="application/pgp-signature"; boundary="pY3vCvL1qV+PayAL"
> Content-Disposition: inline
>
>
> --pY3vCvL1qV+PayAL
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
>
> On Sun, Dec 18, 2005 at 09:32:22AM -0800, Barton C Massey wrote:
> > Of course, my favorite is alternative 3: "let's do both!"
> >=20
> > 3. Provide fooErr() and foo() versions of all *requests*
> > expecting replies. If an application uses fooErr(), it
> > *must* supply a pointer to storage in the extra
> > argument to *Reply() (or XCB will dump core on an
> > assertion). This request will behave as in (1).
> > If it uses foo(), it *must* give 0 in the extra
> > argument to *Reply() (or XCB will dump core on an
> > assertion). This request will behave as in (2).
>
> I'd modify this proposal to pass the XCBGenericError** to fooErr, rather
> than fooReply, because that eliminates the need for these asserts. In
> that sense this is an API extension to my proposal, #2.
>
> And I don't think there's any reason to abbreviate here: it may as well
> be called fooError.
>
> > Further, provide fooErr() versions of requests
> > *not* expecting replies. Provide a function
> >=20
> > int XCBCheckErr(XCBConnection *c,
> > int seqnum,
> > XCBGenericError **e)
> > =09
> > that may only be called with the seqnum of such a
> > request, and *must* be supplied with a pointer to
> > storage in the third argument.
>
> Again I would drop the third argument in favor of passing it to fooErr;
> but this means that CheckErr is just the function that blocks until the
> outcome of a request is known. I keep forgetting that there's been some
> demand for that function.
>
> If CheckErr stayed in the above form, the second argument should be an
> XCBVoidCookie so the type system will enforce that it can only be called
> for void requests.
>
> > (Perhaps further require that the second argument to
> > XCBSync() always be 0, or eliminate it, or make it
> > return only the error from the noop used to do the
> > XCBSync() by using the *Err() request if the pointer
> > is supplied? I'm not sure what its intended purpose
> > is currently.)
>
> I suppose it's never useful to suppress errors from a Sync. It just
> seemed natural not to hide the error argument that's inside the calls
> the Sync function is doing.
>
> > 3 is kind of complicated, but it has some putative
> > advantages. 3 allows one to dump core whenever an error is
> > received by WaitForEvent() while still allowing suppression
> > of "expected" errors even for requests not expecting
> > replies. 3 doesn't modify the existing API in any important
> > way---the only change is that code that tried to use the
> > error field of *Reply() will have to be modified to either
> > use the *Err() requests or to stop doing that. For much of
> > this code that was a good idea anyway, since the results of
> > doing what it was doing are unpredictable.
> >=20
> > What do folks think?
>
> I like #3, given the above changes. I strongly dislike leaving the
> XCBGenericError** argument in the *Reply functions unless it's in the
> style of Keith's proposal, #1 -- which I'm not a big fan of due to the
> asymmetry.
>
> Correct me if I'm wrong, Bart: you feel at this point it's too late to
> make backwards-incompatible changes that affect most of the code that
> uses XCB. I disagree, and I really want a clean API -- enough to
> inconvenience the good people who have been kind enough to test out and
> improve this code for us. I think they will forgive me, but I'd be happy
> to make the necessary changes to their source by hand myself if that's
> what it takes to release with a satisfying API.
>
> --Jamey
>
> --pY3vCvL1qV+PayAL
> Content-Type: application/pgp-signature; name="signature.asc"
> Content-Description: Digital signature
> Content-Disposition: inline
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.2 (GNU/Linux)
>
> iD8DBQFDpvuOp1aplQ4I9mURAidZAKCPVa+8EEJjAkkgSbRsOvfQDHnDawCfU4ar
> mYHXQuCGRBg8awY3KQjP3g4=
> =2Fg1
> -----END PGP SIGNATURE-----
>
> --pY3vCvL1qV+PayAL--
>
> --===============1146270724==
> Content-Type: text/plain; charset="us-ascii"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline
>
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
> --===============1146270724==--
More information about the Xcb
mailing list