[PATCH 2/5] Use correct swap{l,s} (or none at all for CARD8)

walter harms wharms at bfs.de
Fri Aug 5 07:07:13 PDT 2011


hi matt,

I am not sure if i understand the purpose of swap() really.

do you generaly swap bytes ? then you may like a look at swab() (man 3 swab).
On the other side when endianes is a problem ntoh() and friends may be more friendly.

the macro uses "sizeof((src)) != 2" perhaps you can use int16_t here better ?

just my 2 cents,
re,
 wh


Am 05.08.2011 00:06, schrieb Matt Turner:
> Swapping the wrong size was never caught because swap{l,s} are macros.
> 
> It's clear in the case of Xext/xres.c, that the author believed
> client_major/minor to be CARD16 from looking at the code in the first
> hunk.
> 
> Signed-off-by: Matt Turner <mattst88 at gmail.com>
> ---
>  Xext/saver.c                          |    1 -
>  Xext/xres.c                           |   12 ++----------
>  Xi/getdctl.c                          |    2 --
>  Xi/xichangehierarchy.c                |    2 +-
>  Xi/xiquerydevice.c                    |    2 +-
>  hw/xfree86/dixmods/extmod/xf86vmode.c |    4 ++--
>  randr/rrcrtc.c                        |    4 ++--
>  xkb/xkb.c                             |    1 -
>  8 files changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/Xext/saver.c b/Xext/saver.c
> index 9e91b71..142758c 100644
> --- a/Xext/saver.c
> +++ b/Xext/saver.c
> @@ -1443,7 +1443,6 @@ SProcScreenSaverSuspend (ClientPtr client)
>  
>      swaps(&stuff->length);
>      REQUEST_SIZE_MATCH(xScreenSaverSuspendReq);
> -    swapl(&stuff->suspend);
>      return ProcScreenSaverSuspend (client);
>  }
>  
> diff --git a/Xext/xres.c b/Xext/xres.c
> index 9df12ae..b952728 100644
> --- a/Xext/xres.c
> +++ b/Xext/xres.c
> @@ -28,15 +28,9 @@ ProcXResQueryVersion (ClientPtr client)
>  {
>      REQUEST(xXResQueryVersionReq);
>      xXResQueryVersionReply rep;
> -    CARD16 client_major, client_minor;  /* not used */
>  
>      REQUEST_SIZE_MATCH (xXResQueryVersionReq);
>  
> -    client_major = stuff->client_major;
> -    client_minor = stuff->client_minor;
> -    (void) client_major;
> -    (void) client_minor;
> -
>      rep.type = X_Reply;
>      rep.length = 0;
>      rep.sequenceNumber = client->sequence;
> @@ -316,8 +310,6 @@ SProcXResQueryVersion (ClientPtr client)
>  {
>      REQUEST(xXResQueryVersionReq);
>      REQUEST_SIZE_MATCH (xXResQueryVersionReq);
> -    swaps(&stuff->client_major);
> -    swaps(&stuff->client_minor);
>      return ProcXResQueryVersion(client);
>  }
>  
> @@ -326,7 +318,7 @@ SProcXResQueryClientResources (ClientPtr client)
>  {
>      REQUEST(xXResQueryClientResourcesReq);
>      REQUEST_SIZE_MATCH (xXResQueryClientResourcesReq);
> -    swaps(&stuff->xid);
> +    swapl(&stuff->xid);
>      return ProcXResQueryClientResources(client);
>  }
>  
> @@ -335,7 +327,7 @@ SProcXResQueryClientPixmapBytes (ClientPtr client)
>  {
>      REQUEST(xXResQueryClientPixmapBytesReq);
>      REQUEST_SIZE_MATCH (xXResQueryClientPixmapBytesReq);
> -    swaps(&stuff->xid);
> +    swapl(&stuff->xid);
>      return ProcXResQueryClientPixmapBytes(client);
>  }
>  
> diff --git a/Xi/getdctl.c b/Xi/getdctl.c
> index 4287028..6090b81 100644
> --- a/Xi/getdctl.c
> +++ b/Xi/getdctl.c
> @@ -127,7 +127,6 @@ static void CopySwapDeviceCore (ClientPtr client, DeviceIntPtr dev, char *buf)
>      if (client->swapped) {
>          swaps(&c->control);
>          swaps(&c->length);
> -        swaps(&c->status);
>      }
>  }
>  
> @@ -142,7 +141,6 @@ static void CopySwapDeviceEnable (ClientPtr client, DeviceIntPtr dev, char *buf)
>      if (client->swapped) {
>          swaps(&e->control);
>          swaps(&e->length);
> -        swaps(&e->enable);
>      }
>  }
>  
> diff --git a/Xi/xichangehierarchy.c b/Xi/xichangehierarchy.c
> index f2bd8bb..614d231 100644
> --- a/Xi/xichangehierarchy.c
> +++ b/Xi/xichangehierarchy.c
> @@ -434,7 +434,7 @@ ProcXIChangeHierarchy(ClientPtr client)
>      any = (xXIAnyHierarchyChangeInfo*)&stuff[1];
>      while(stuff->num_changes--)
>      {
> -        SWAPIF(swapl(&any->type));
> +        SWAPIF(swaps(&any->type));
>          SWAPIF(swaps(&any->length));
>  
>          required_len += any->length;
> diff --git a/Xi/xiquerydevice.c b/Xi/xiquerydevice.c
> index f5fca0d..902eb91 100644
> --- a/Xi/xiquerydevice.c
> +++ b/Xi/xiquerydevice.c
> @@ -281,7 +281,7 @@ SwapButtonInfo(DeviceIntPtr dev, xXIButtonInfo* info)
>      swaps(&info->sourceid);
>  
>      for (i = 0, btn = (Atom*)&info[1]; i < info->num_buttons; i++, btn++)
> -        swaps(btn);
> +        swapl(btn);
>  
>      swaps(&info->num_buttons);
>  }
> diff --git a/hw/xfree86/dixmods/extmod/xf86vmode.c b/hw/xfree86/dixmods/extmod/xf86vmode.c
> index 46ff3bf..6d3d5fc 100644
> --- a/hw/xfree86/dixmods/extmod/xf86vmode.c
> +++ b/hw/xfree86/dixmods/extmod/xf86vmode.c
> @@ -464,7 +464,7 @@ ProcXF86VidModeGetAllModeLines(ClientPtr client)
>  	    swaps(&mdinf.hsyncstart);
>  	    swaps(&mdinf.hsyncend);
>  	    swaps(&mdinf.htotal);
> -	    swaps(&mdinf.hskew);
> +	    swapl(&mdinf.hskew);
>  	    swaps(&mdinf.vdisplay);
>  	    swaps(&mdinf.vsyncstart);
>  	    swaps(&mdinf.vsyncend);
> @@ -1846,7 +1846,7 @@ SProcXF86VidModeSwitchToMode(ClientPtr client)
>      REQUEST(xXF86VidModeSwitchToModeReq);
>      swaps(&stuff->length);
>      REQUEST_SIZE_MATCH(xXF86VidModeSwitchToModeReq);
> -    swaps(&stuff->screen);
> +    swapl(&stuff->screen);
>      return ProcXF86VidModeSwitchToMode(client);
>  }
>  
> diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
> index a8b73d9..e6a38ae 100644
> --- a/randr/rrcrtc.c
> +++ b/randr/rrcrtc.c
> @@ -1117,7 +1117,7 @@ ProcRRGetPanning (ClientPtr client)
>      if (client->swapped) {
>  	swaps(&rep.sequenceNumber);
>  	swapl(&rep.length);
> -	swaps(&rep.timestamp);
> +	swapl(&rep.timestamp);
>  	swaps(&rep.left);
>  	swaps(&rep.top);
>  	swaps(&rep.width);
> @@ -1197,7 +1197,7 @@ sendReply:
>      if (client->swapped) {
>  	swaps(&rep.sequenceNumber);
>  	swapl(&rep.length);
> -	swaps(&rep.newTimestamp);
> +	swapl(&rep.newTimestamp);
>      }
>      WriteToClient(client, sizeof(xRRSetPanningReply), (char *)&rep);
>      return Success;
> diff --git a/xkb/xkb.c b/xkb/xkb.c
> index 9ae9b9e..8e42a7f 100644
> --- a/xkb/xkb.c
> +++ b/xkb/xkb.c
> @@ -6190,7 +6190,6 @@ char *			str;
>  	swaps(&rep.supported);
>  	swaps(&rep.unsupported);
>  	swaps(&rep.nDeviceLedFBs);
> -	swapl(&rep.type);
>      }
>      WriteToClient(client,SIZEOF(xkbGetDeviceInfoReply), (char *)&rep);
>  


More information about the xorg-devel mailing list