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

[Dan Nicholson]

On Oct 27, 2011 4:26 AM, "Peter Hutterer" <peter.hutterer at who-t.net> wrote:
>
> 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.

Drive by reviewing ...
What about an assert? Than the user's incorrect program will at least
stop with a usefulish message. Seems better then a segfault.

--
Dan