[RFC PATCH] gen_swap_check handles xproto and shape extensions

Peter Hutterer peter.hutterer at who-t.net
Mon Mar 16 23:30:09 PDT 2015


On Wed, Mar 11, 2015 at 03:22:07PM +0100, Christian Linhart wrote:
> Hi Asalle,
> 
> Thank you for posting your changes as a patch.
> (Next time, please do not sent it as attchment but in-message, because this is easier to handle for reviewers.
> use git send-email for posting a patch)
> 
> Anyways, below is a quick review of your patch,
> mostly focusing on stuff about patch-generation.
> 
> My comments are inserted below within the quoted version of your patch.
> 
> Please fix these things and regenerate a new version of your patch.
> 
[...]
> > +
> > +//static int
> > +//SProcShapeDispatch(ClientPtr client)
> > +//{
> > +    //REQUEST(xReq);
> > +    //switch (stuff->data) {
> > +    //case X_ShapeQueryVersion:
> > +        //return SProcShapeQueryVersion(client);
> > +    //case X_ShapeRectangles:
> > +        //return SProcShapeRectangles(client);
> > +    //case X_ShapeMask:
> > +        //return SProcShapeMask(client);
> > +    //case X_ShapeCombine:
> > +        //return SProcShapeCombine(client);
> > +    //case X_ShapeOffset:
> > +        //return SProcShapeOffset(client);
> > +    //case X_ShapeQueryExtents:
> > +        //return SProcShapeQueryExtents(client);
> > +    //case X_ShapeSelectInput:
> > +        //return SProcShapeSelectInput(client);
> > +    //case X_ShapeInputSelected:
> > +        //return SProcShapeInputSelected(client);
> > +    //case X_ShapeGetRectangles:
> > +        //return SProcShapeGetRectangles(client);
> > +    //default:
> > +        //return BadRequest;
> > +    //}
> > +//}
> 
> Please remove these commented-out things.
> There is no need to keep old code in this way.
> The old is accessible anyways in the git-history.

also, for the future: 
// is a c++ comment, we don't use them in the server. Use /* */ instead,
but if you want to disable a whole block of code at once the best approach
is usually
#if 0
some code
#endif

if you want to get really fancy you can do things like

#if SOMETHING
#endif

without ever defining SOMETHING, that way you can later get rid of it in one
go using the unifdef tool. that's useful when you want to disable a whole
bunch of things in multiple files.

Cheers,
   Peter
 


More information about the xorg-devel mailing list