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
signature.asc
Description: Digital signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
