Maintaining backwards compatibility with swap macros

[Jeremy Huddleston]

ping.  I didn't see anything as a followup to my comments below.  I'm fine with either approach, but I'd like to get this in soon.  

On Sep 25, 2011, at 11:22 AM, Jeremy Huddleston wrote:

> 
> On Sep 25, 2011, at 10:50, Matt Turner wrote:
> 
>> Dave pointed out that there are a couple drivers (sis, sisusb, vmware)
>> that use the swapl/swaps macros. My recent patch series dropped the n
>> argument from the macros, causing these drivers to not build.
>> 
>> Ideally, we'd like a deprecation warning when the second argument is
>> given, but by removing the second argument, we'd lose compatibility with
>> old servers.
>> 
>> We could modify the swap macros in the server with the following patch,
>> or we could update the drivers to use a single argument
>> #if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1, 11, 99, 0).
>> 
>> Thoughts?
> 
> Changing to a vararg macro seems icky to me.
> 
> I'd rather fix these in the drivers themselves instead.  Perhaps bring in my ABI bump patch now rather than waiting for all the bus layer cleanup and use that in the drivers to determine what they should do.  At minimum, they can do:
> 
> find . -type f | xargs sed -i -e 's/swaps/_swaps/g' -e 's/swapl/_swapl/g'
> 
> Then add to some header:
> 
> #if GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 12
> #define _swapl(x, n) swapl(x,n)
> #define _swaps(x, n) swaps(x,n)
> #else
> #define _swapl(x, n) swapl(x)
> #define _swaps(x, n) swaps(x)
> #endif
> 
> If this feels "ickier" than the vararg, I'm really ok with either way.
> 
>> 
>> Matt
>> 
>> diff --git a/include/misc.h b/include/misc.h
>> index 1fea73e..8328555 100644
>> --- a/include/misc.h
>> +++ b/include/misc.h
>> @@ -285,7 +285,7 @@ static inline void swap_uint32(uint32_t *x)
>> 	((char *) x)[2] = n;
>> }
>> 
>> -#define swapl(x) do { \
>> +#define swapl(x, ...) do { \
>> 		if (sizeof(*(x)) != 4) \
>> 			wrong_size(); \
>> 		if (__builtin_constant_p((uintptr_t)(x) & 3) && ((uintptr_t)(x) & 3) == 0) \
>> @@ -302,7 +302,7 @@ static inline void swap_uint16(uint16_t *x)
>> 	((char *) x)[1] = n;
>> }
>> 
>> -#define swaps(x) do { \
>> +#define swaps(x, ...) do { \
>> 		if (sizeof(*(x)) != 2) \
>> 			wrong_size(); \
>> 		if (__builtin_constant_p((uintptr_t)(x) & 1) && ((uintptr_t)(x) & 1) == 0) \
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

---
Jeremy Huddleston

Rebuild Sudan
 - Board of Directors
 - http://www.rebuildsudan.org

Berkeley Foundation for Opportunities in Information Technology
 - Advisory Board
 - http://www.bfoit.org