On Tue, 8 Nov 2011 10:47:44 -0800, Jamey Sharp <[email protected]> wrote:

> I feel like this should turn into "if (client && client->requestBuffer)"
> rather than hiding the real condition inside the REQUEST macro.

I think that's a separate cleanup, and there are lots of such possible :-)

> > +    major = ((xReq *)client->requestBuffer)->reqType;
> > +    if (major < EXTENSION_BASE)
> > +   return 0;
> 
> GetExtensionEntry already checks that major is at least EXTENSION_BASE,
> so this test is redundant.

True; dunno whether we care about the cost of a function call for even
core requests though?

> And can I persuade you to just inline this directly into its only call
> site, below, in Dispatch? I don't think factoring it out enhances
> clarity in this case, especially since this way you need to look up the
> major opcode in both places.

Could use client->majorOp inside the function. In any case, it's a
static inline now, so one presumes the compiler would dtrt to some
degree?

Is it OK to remove the MinorOpcodeOfRequest function from the API?
Should it be preserved, but re-implemented using client->minorOp?

-- 
[email protected]

Attachment: pgpHTkKqxg82u.pgp
Description: PGP signature

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to