[Pixman] [PATCH v2] pixman-general: Tighten up calculation of temporary buffer sizes

[Pekka Paalanen]

On Tue, 22 Sep 2015 12:43:25 +0100
Ben Avison <bavison at riscosopen.org> wrote:

> Each of the aligns can only add a maximum of 15 bytes to the space
> requirement. This permits some edge cases to use the stack buffer where
> previously it would have deduced that a heap buffer was required.
> ---
> 
> This is an update of my previous patch (now posted over a year ago):
> https://patchwork.freedesktop.org/patch/49898/
> 
> which conflicts with the patch just pushed:
> http://patchwork.freedesktop.org/patch/60052/
> 
> Since this piece of code is fresh in people's minds, this might be a good
> time to get this one pushed as well.
> 
> Note that the scope of this change has been reduced: while the old code
> added space to align the end address to a cacheline boundary (which as I
> pointed out, was unnecessary), the new version works using buffer lengths
> only.
> 
> As a discussion point, wouldn't it be better for the ALIGN macro to
> assume 32-byte cache lines? Both 16-byte and 32-byte cachelines are
> currently in common use, but by aligning the buffers to 32-byte addresses
> we would simultaneously achieve 16-byte alignment.
> 
>  pixman/pixman-general.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/pixman/pixman-general.c b/pixman/pixman-general.c
> index fa88463..6141cb0 100644
> --- a/pixman/pixman-general.c
> +++ b/pixman/pixman-general.c
> @@ -158,9 +158,9 @@ general_composite_rect  (pixman_implementation_t *imp,
>      if (width <= 0 || _pixman_multiply_overflows_int (width, Bpp * 3))
>  	return;
>  
> -    if (width * Bpp * 3 > sizeof (stack_scanline_buffer) - 32 * 3)
> +    if (width * Bpp * 3 > sizeof (stack_scanline_buffer) - 15 * 3)
>      {
> -	scanline_buffer = pixman_malloc_ab_plus_c (width, Bpp * 3, 32 * 3);
> +	scanline_buffer = pixman_malloc_ab_plus_c (width, Bpp * 3, 15 * 3);
>  
>  	if (!scanline_buffer)
>  	    return;

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

I think Ben's explanation as seen in
https://patchwork.freedesktop.org/patch/49898/
covers all Søren's concerns (it quotes everything Søren said about the
patch), and I see no reason to reject this.

I'll push this on Friday if no-one objects.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20150922/f5c5eddc/attachment.sig>