[PATCH libX11 v2 1/4] Add _XGetRequest as substitute for GetReq/GetReqExtra

Peter Hutterer peter.hutterer at who-t.net
Thu Oct 27 04:25:56 PDT 2011


On Thu, Oct 27, 2011 at 10:41:06AM +0200, walter harms wrote:
> 
> 
> Am 27.10.2011 09:55, schrieb Jamey Sharp:
> > On Thu, Oct 27, 2011 at 09:15:54AM +0200, walter harms wrote:
> >> Am 27.10.2011 06:21, schrieb Peter Hutterer:
> >>> +void *_XGetRequest(Display *dpy, CARD8 type, size_t len)
> >>> +{
> >>> +    xReq *req;
> >>> +
> >>> +    WORD64ALIGN
> >>> +
> >>> +    if (dpy->bufptr + len > dpy->bufmax)
> >>> +	_XFlush(dpy);
> >>> +
> >>> +    if (len % 4)
> >>> +	fprintf(stderr,
> >>> +		"Xlib: request %d length %zd not a multiple of 4.\n",
> >>> +		type, len);
> >>
> >> Does it make sense to continue here ?
> >> perhaps you want a add a return NULL ?
> > 
> > It doesn't make sense to continue, but there's no way to report the
> > error that any caller can handle. If you return NULL here, the caller is
> > guaranteed to segfault.
> > 
> > Since these errors are already possible today, but aren't being even
> > noticed, I think Peter's choice of a printf is the best we can do. At
> > least it allows the possibility of somebody noticing the bug.
> > 
> > It'd be nice if we could get more information than the minor opcode for
> > extension requests, but nothing else is immediately obvious to me here.
> > 
> what is bad with segfault ? the macro version would more or less make
> sure that len % 4 is 0 by using sizeof. Everybody else has a serious problem.
> Better you stop here (by returning NULL) than at a random place.

all the existing code still uses the macros so the same assumptions are
true. Only new code could use _XGetRequest directly and you'd hope that
whoever writes that code runs it at least once to see the error message.

Cheers,
  Peter


More information about the xorg-devel mailing list