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

[Fernando Carrijo]

Jamey Sharp <jamey at minilop.net> wrote:

> On Wed, Jul 28, 2010 at 05:26:34PM -0300, Fernando Carrijo wrote:
> > ---
> > 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. :-)

Okay.

> 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".

Yes.

> > + *   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.

No problem. I will respin this patch as a reviewer-friendly series.

> 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.

Those modifications are just a collateral effect of me trying to understand the
context in which Tiago's input thread lives. And although I feel fascinated by
this part of the server, I'm certainly ignorant about all the corner cases the
current implementation of the smart scheduler deals with.

Nevertheless, things get definitely easier for me when more knowledgeable people
do as you did, and comment about other's short-sighted views. To the extent that
I could consider investigating this question more closely and possibly contribute
with more substance in this regard.

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

Right.

Thanks for reviewing, Jamey.