[PATCH] dix: Better document the functions Dispatch(), WaitForSomething() and alike.

Jamey Sharp jamey at minilop.net
Wed Jul 28 21:14:55 PDT 2010


On Wed, Jul 28, 2010 at 05:26:34PM -0300, Fernando Carrijo wrote:
> Signed-off-by: Fernando Carrijo <fcarrijo at yahoo.com.br>
> ---
> Wrote this patch more as an exercise than anything else. And while I
> realize that there's a thin line between what people consider benign
> and pernicious commenting styles, I tend to believe that in case of
> intricate code such as this, we better sin for verbiage than laconism.
> 
> Some words I took straight from "Efficiently Scheduling X Clients",
> for which I hope not to be sued for plagiarism. Others are my own, and
> I'm still not in position to guarantee correctness.

I'd suggest including the above text in the commit message. :-)

Also, a small typo:

> --- a/dix/dispatch.c
> +++ b/dix/dispatch.c
> @@ -240,6 +240,14 @@ long	    SmartLastPrint;
>  void        Dispatch(void);
>  void        InitProcVectors(void);
>  
> +/* clientReady:
> + *   array of clients ready for input and/or output. Actually, the
> + *   array is not composed of clients, but of the indexes they occupy
> + *   in the global variable clients.
> + *
> + * nread:

Apparently you meant "nready".

> + *   number of items in the array
> + */
>  static int
>  SmartScheduleClient (int *clientReady, int nready)
>  {

I found it harder to review this patch because of the whitespace changes
mixed in. In the future I'd suggest perhaps doing the reformatting
first, and the new documentation afterward. I don't think that's worth
splitting this patch for though.

I'm not familiar with this part of the code, and in fact you may now be
the world expert in X server request scheduling. Sucks to be you. :-P
Will we be seeing more patches from you on this code in the near future?
I bet you've spotted some way you could simplify it.

Feel free to add my
  Reviewed-by: Jamey Sharp <jamey at minilop.net>
to this patch, especially if you make the changes I noted.

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/20100728/33d72b58/attachment.pgp>


More information about the xorg-devel mailing list