[Pixman] [RFC PATCH 2/3] utils.[ch]: add fence_image_create_bits ()

Ben Avison bavison at riscosopen.org
Wed May 13 16:25:45 PDT 2015


On Fri, 08 May 2015 13:45:36 +0100, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> +pixman_image_t *
> +fence_image_create_bits (pixman_format_code_t format,
> +                         int min_width, int height,
> +                         pixman_bool_t stride_fence)

Since you picked me up on subtleties of coding style, it's only fair that
I point out that in function definitions, the arguments should either all
be on one line, or one per line (with identifier names justified).

> +    pixels = fence_malloc (stride * (unsigned)height);

I wonder if it wouldn't be better to reduce the size here by one page if
stride_fence==false - otherwise you've got a gap of one page between the
end of the last row of pixel data and the end fence.

> +pixman_image_t *
> +fence_image_create_bits (pixman_format_code_t format,
> +                         int min_width, int height,
> +                         pixman_bool_t stride_fence)
> +{
> +    return pixman_image_create_bits (format, width, height, NULL, 0);
> +}

Perhaps a comment here to say that the auto-malloced pixel array is also
auto-freed when the image reference count drops to zero, so there's no
need for a destroy function in this case?

Ben


More information about the Pixman mailing list