[PATCH 1/5] Use internal temp variable for swap macros

Matt Turner mattst88 at gmail.com
Mon Aug 15 15:37:47 PDT 2011


On Thu, Aug 4, 2011 at 6:11 PM, Matt Turner <mattst88 at gmail.com> wrote:
> This patch was way too big to make it to the mailing list, so you can
> see it here
>
> http://cgit.freedesktop.org/~mattst88/xserver/commit/?h=bswap-cleanup&id=039d90b2a6a68496ef63f4b7e557c41b3bc82379
>
> I used Coccinelle and a simple semantic patch to remove the second
> argument from swap[ls]:
> ---
> @@
> expression x, n;
> @@
>
> - swapl(x, n)
> + swapl(x)
>
> @@
> expression x, n;
> @@
>
> - swaps(x, n)
> + swaps(x)
> ---
>
> Left over 'register int n'-type variables were removed by hand, so
> that no unused variable warnings were added.
>
> Matt

Anyone want to review this series?

Patch 1: mostly Coccinelle-automated, with some manual left over
variable removals.
Patch 2: fixes incorrect swaps (wrong size swap() used, or tried to
swap an 8-bit value), found and fixed after Patch 3
Patch 3: adds type checking to swap macros and casts remaining
type-mismatches (byte-swapping elements in char* buffers)
Patch 4: use lswapl() in cpswapl(), and fix indention of cpswap*()
Patch 5: fix indention of swap*()

It's been pointed out that I could use sizeof(type) in the
type-checking. I don't feel strongly about this.

I think it may be cleaner to move all the indention/whitespace fixes
in patch 4 into patch 5.

The changes to xkb/xkb.c in patch 3 could perhaps be made cleaner, ie,
changing the type of 'desc' to int or CARD32. Perhaps something should
be done to ensure that the memory calloc()d to desc* is 4-byte
aligned. Anyway, patch 3 needs review the most, specifically for
alignment problems.

I don't see anything wrong with the first two patches.

Matt


More information about the xorg-devel mailing list