[OPW] Generated swapping and size-checking code integration

[chris at demorecorder.com]

Hi Asalle and all,

I am Asalle's OPW-mentor.

Asalle, Thank you for your intro and for posting the result of your past 2+ months of work.

The newest code in Asalle's repo at
https://webmail.all-inkl.com/bitbucket.org/AsalleKim/gen_swap_check
is in the branch "basic_swapping", so please check out that branch
for testing/reviewing.

For now, I have a comment to your question 2, 
and a quick review of the integration into Xext/shape.c, and one comment about the dispatch functions. 
( I know most of the other code already... though I didn't test your latest changes.)

Asal Mirzaieva wrote on 16.02.2015 20:46:
> 2. To test the code I need to integrate it to the xserver. 
> I already made some changes into Xext/shape.c so it uses the generated code now. 
> The Xext/shape.h must include the swapcheck_shape.h (generated one). 
> Would it be okay if I copy all generated headers to {installdir}/include/xcb, 
> mixing the genswap-generated code and the regular code, generated by c_client.py? 
> And copy the compiled swapcheck_* files to {installdir}/lib.

You do not need to go via {installdir} for these files because everything is internal to xorg/xserver.
Actually, these files should not be installed because they do not represent an external API. 

I think it will be sufficient to add the directory containing 
the generated headers to the includepath for those *.c files
which include them.
( the headers are generated into subdirectory "proto/gen" of xorg/xserver )

Or put the generated headers in the "include" directory of the xorg/xserver source
(or a subdirectory of it.)

In general, I cannot provide good help for the build-specific issues
because Automake is not yet an area of strength. 
(I learned most of my Xserver stuff when imake was still used...)

I'd appreciate help in this area from people on this list who
are more knowledgeable with the Automake based build system.
 
***

Here's my review of your changes to Xext/shape.c:

> diff --git a/Xext/shape.c b/Xext/shape.c
> index bb479b1..54e71d6 100644
> --- a/Xext/shape.c
> +++ b/Xext/shape.c
> @@ -1025,9 +1025,25 @@ ProcShapeGetRectangles(ClientPtr client)
>      return Success;
>  }
>  
> +#include "xcb/swapcheck_shape.h"
> +
>  static int
>  ProcShapeDispatch(ClientPtr client)
>  {
> +	REQUEST(xReq);
> +	if(xcb_shape_Check_dispatch(client) == Success)
This should be "stuff" instead of "client" because your function processes a request-ptr directly.
The return-type of xcb_shape_Check_dispatch is not a BOOL 
but a status. So you'd need to add "== Success".


> +	{
> +		return ProcShapeDispatch_Unchecked(client);
> +	}
> +	else
> +	{
> +		return BadLength;
> +	}
> +}
> +
> +static int
> +ProcShapeDispatch_Unchecked(ClientPtr client)
> +{
>      REQUEST(xReq);
>      switch (stuff->data) {
>      case X_ShapeQueryVersion:
> @@ -1073,6 +1089,22 @@ ProcShapeDispatch(ClientPtr client)
>      }
>  }
>  
> +static int
> +SProcShapeDispatch(ClientPtr client)
> +{
> +    REQUEST(xReq);
> +    
> +    if(xcb_shape_SwapFromClient_dispatch(client))
This should be "stuff" instead of "client".
The return-type of xcb_shape_SwapFromClient_dispatch is not a BOOL 
but a status. So you'd need to add "== Success".

> +    {
> +		return ProcShapeDispatch_Unchecked(client);
> +	}
> +	else
> +	{
> +		return BadLength;
> +	}
> +}
> +
> +
>  static void
>  SShapeNotifyEvent(xShapeNotifyEvent * from, xShapeNotifyEvent * to)
>  {
> -- 
> 2.1.1

You'll need to remove the original function SProcShapeDispatch,
and all the other SProc functions.

***

In the generated dispatch functions, there should be 
a "default:" label in the switch statements.
For "default:", return a "BadRequest" error code.

Best regards,

Chris