[PATCH] Save major/minor opcodes in ClientRec for RecordAReply

[Jamey Sharp]

You're welcome to throw my reviewed-by on this patch, with the caveat
that not only have I not tested it, but I haven't investigated the issue
it's trying to address, either. But it seems like an improvement to the
code regardless of any bugs it might fix.

I'd suggest a couple minor edits, though:

On Tue, Nov 08, 2011 at 10:22:13AM -0800, Keith Packard wrote:
> diff --git a/Xext/xselinux_hooks.c b/Xext/xselinux_hooks.c
> index f1d8e5d..0d4c9ab 100644
> --- a/Xext/xselinux_hooks.c
> +++ b/Xext/xselinux_hooks.c
> @@ -263,8 +263,8 @@ SELinuxAudit(void *auditdata,
>      if (client) {
>  	REQUEST(xReq);
>  	if (stuff) {

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

> -	    major = stuff->reqType;
> -	    minor = MinorOpcodeOfRequest(client);
> +	    major = client->majorOp;
> +	    minor = client->minorOp;
>  	}
>      }
>      if (audit->id)
> diff --git a/dix/dispatch.c b/dix/dispatch.c
> index 6e33615..3600acd 100644
> --- a/dix/dispatch.c
> +++ b/dix/dispatch.c
> @@ -337,7 +337,20 @@ DisableLimitedSchedulingLatency(void)
>  	SmartScheduleLatencyLimited = 0;
>  }
>  
> -#define MAJOROP ((xReq *)client->requestBuffer)->reqType
> +static inline unsigned short
> +MinorOpcodeOfRequest(ClientPtr client)
> +{
> +    unsigned char major;
> +    ExtensionEntry *ext;
> +
> +    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.

> +    ext = GetExtensionEntry(major);
> +    if (!ext)
> +	return 0;
> +    return ext->MinorOpcode (client);
> +}

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.

>  void
>  Dispatch(void)
> @@ -419,21 +432,23 @@ Dispatch(void)
>  	        }
>  
>  		client->sequence++;
> +		client->majorOp = ((xReq *)client->requestBuffer)->reqType;
> +		client->minorOp = MinorOpcodeOfRequest(client);

Jamey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20111108/00ddf07b/attachment.pgp>