[PATCH] Define/use X_ALIGN2() instead of open coding it

Matt Turner mattst88 at gmail.com
Sat Jun 19 15:37:23 PDT 2010


On Sat, Jun 19, 2010 at 6:32 PM, Mikhail Gusarov
<dottedmag at dottedmag.net> wrote:
>
> Twas brillig at 18:14:31 19.06.2010 UTC-04 when mattst88 at gmail.com did gyre and gimble:
>
>  MT> Signed-off-by: Matt Turner <mattst88 at gmail.com>
>
> From the discussion in IRC:
>
> - X_ALIGN2 is not clear enough. pad_to_pow_two?

Fine by me, but I sent a nearly identical patch months ago with this name:

On Wed, Mar 24, 2010 at 2:56 PM, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
>> From: Matt Turner <mattst88 at gmail.com>
>> Date: Wed, 24 Mar 2010 13:57:15 -0400
>>
>> diff --git a/hw/dmx/dmxpict.c b/hw/dmx/dmxpict.c
>> index 072e3a6..51616bb 100644
>> --- a/hw/dmx/dmxpict.c
>> +++ b/hw/dmx/dmxpict.c
>> @@ -674,7 +674,7 @@ static int dmxProcRenderSetPictureFilter(ClientPtr client)
>>
>>      if (pPictPriv->pict) {
>>       filter  = (char *)(stuff + 1);
>> -     params  = (XFixed *)(filter + ((stuff->nbytes + 3) & ~3));
>> +     params  = (XFixed *)(filter + pad_to_pow_two(stuff->nbytes, 4));
>
> Sorry, but to me this isn't an improvement.  I probably spend to much
> time on kernel hacking, but the origional is immediately obvious to
> me, whereas the new line makes me think you're trying to align to a
> 16-byte boundary.

Maybe I should replace all (x + 3) & ~3 with pad_to_int32 first, and
then everything else be changed to pad_to_pow_two?

> - It's not macro, so no need to have it all caps.
> - misc.h brings too much garbage, better have include/align.h,
> - So _HAVE_XALLOC_DECLS won't be needed.

The rest sounds fine to me. Any objections to creating an align.h that
we can move the pad_to_* functions into?

Thanks,
Matt


More information about the xorg-devel mailing list