[2/2] render: Fix byteswapping of gradient stops

Submitted by Andrea Canciani on Nov. 3, 2010, 2:10 a.m.

Details

Message ID 1288725032-684-2-git-send-email-ranma42@gmail.com
State Accepted, archived
Commit dab064fa5e0b1f5c67222562ad5367005832cba1
Headers show

Not browsing as part of any series.

Commit Message

Andrea Canciani Nov. 3, 2010, 2:10 a.m.
The function swapStops repeatedly swaps the color components as
CARD16, but incorrectly steps over them as if they were CARD32.

This causes half of the stops not to be swapped at all and some
unrelated data be swapped instead.
---
 render/render.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/render/render.c b/render/render.c
index 00241f9..85a4392 100644
--- a/render/render.c
+++ b/render/render.c
@@ -2552,8 +2552,8 @@  static void swapStops(void *stuff, int num)
     }
     colors = (CARD16 *)(stops);
     for (i = 0; i < 4*num; ++i) {
-        swaps(stops, n);
-        ++stops;
+        swaps(colors, n);
+        ++colors;
     }
 }
 

Comments

Andrea Canciani <ranma42@gmail.com> writes:

> The function swapStops repeatedly swaps the color components as
> CARD16, but incorrectly steps over them as if they were CARD32.
> 
> This causes half of the stops not to be swapped at all and some
> unrelated data be swapped instead.
> ---
>  render/render.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/render/render.c b/render/render.c
> index 00241f9..85a4392 100644
> --- a/render/render.c
> +++ b/render/render.c
> @@ -2552,8 +2552,8 @@ static void swapStops(void *stuff, int num)
>      }
>      colors = (CARD16 *)(stops);
>      for (i = 0; i < 4*num; ++i) {
> -        swaps(stops, n);
> -        ++stops;
> +        swaps(colors, n);
> +        ++colors;
>      }
>  }

Reviewed-by: Soren Sandmann <sandmann@daimi.au.dk>


Soren
On Thu, Nov  4, 2010 at 02:44:48 +0100, Soeren Sandmann wrote:

> Andrea Canciani <ranma42@gmail.com> writes:
> 
> > The function swapStops repeatedly swaps the color components as
> > CARD16, but incorrectly steps over them as if they were CARD32.
> > 
> > This causes half of the stops not to be swapped at all and some
> > unrelated data be swapped instead.
> > ---
> >  render/render.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/render/render.c b/render/render.c
> > index 00241f9..85a4392 100644
> > --- a/render/render.c
> > +++ b/render/render.c
> > @@ -2552,8 +2552,8 @@ static void swapStops(void *stuff, int num)
> >      }
> >      colors = (CARD16 *)(stops);
> >      for (i = 0; i < 4*num; ++i) {
> > -        swaps(stops, n);
> > -        ++stops;
> > +        swaps(colors, n);
> > +        ++colors;
> >      }
> >  }
> 
> Reviewed-by: Soren Sandmann <sandmann@daimi.au.dk>
> 
Reviewed-by: Julien Cristau <jcristau@debian.org>

Keith, can you take this one?  It would be nice to have it on the stable
branches.

Cheers,
Julien
On Tue,  2 Nov 2010 20:10:32 +0100, Andrea Canciani <ranma42@gmail.com> wrote:
> The function swapStops repeatedly swaps the color components as
> CARD16, but incorrectly steps over them as if they were CARD32.
> 
> This causes half of the stops not to be swapped at all and some
> unrelated data be swapped instead.

Missing a Signed-off-by: line here...
On Thu, Dec 2, 2010 at 1:35 AM, Keith Packard <keithp@keithp.com> wrote:
> On Tue,  2 Nov 2010 20:10:32 +0100, Andrea Canciani <ranma42@gmail.com> wrote:
>> The function swapStops repeatedly swaps the color components as
>> CARD16, but incorrectly steps over them as if they were CARD32.
>>
>> This causes half of the stops not to be swapped at all and some
>> unrelated data be swapped instead.
>
> Missing a Signed-off-by: line here...

Right, I should have read
http://www.x.org/wiki/Development/Documentation/SubmittingPatches
before posting.

Signed-off-by: Andrea Canciani <ranma42@gmail.com>

(or should I post it again with the signed-off-by and reviewed-by tags?)

Andrea
On Thu, 2 Dec 2010 09:51:50 +0100, Andrea Canciani <ranma42@gmail.com> wrote:

> (or should I post it again with the signed-off-by and reviewed-by
> tags?)

Nah, I'm pretty good at pasting mails together.
On Wed, 1 Dec 2010 23:59:04 +0100, Julien Cristau <jcristau@debian.org> wrote:
> On Thu, Nov  4, 2010 at 02:44:48 +0100, Soeren Sandmann wrote:
> > Andrea Canciani <ranma42@gmail.com> writes:

> > >      }
> > >      colors = (CARD16 *)(stops);
> > >      for (i = 0; i < 4*num; ++i) {
> > > -        swaps(stops, n);
> > > -        ++stops;
> > > +        swaps(colors, n);
> > > +        ++colors;
> > >      }
> > >  }
> > 
> > Reviewed-by: Soren Sandmann <sandmann@daimi.au.dk>
> > 
> Reviewed-by: Julien Cristau <jcristau@debian.org>
> 
> Keith, can you take this one?  It would be nice to have it on the stable
> branches.

Merged.
   279ef1f..dab064f  master -> master

Note that I have *not* merged the patch which allows for single-stop
gradients -- that awaits autoconf changes to require a fixed version of
pixman. Soren's question about whether the Render protocol version
should be bumped is also valid, but I think that would require a change
to the protocol package as well as the X server.