[Qemu-devel] Re: [PATCH 1/7] Consolidate DisplaySurface allocation in qemu_alloc_display()

[Anthony Liguori]

On 03/15/2011 07:36 AM, address@hidden wrote:
> From: Jes Sorensen<address@hidden>
>
> This removes various code duplication from console.e and sdl.c
>
> Signed-off-by: Jes Sorensen<address@hidden>
> ---
>   console.c |   45 +++++++++++++++++++++++++--------------------
>   console.h |    3 +++
>   ui/sdl.c  |   21 ++++++++-------------
>   3 files changed, 36 insertions(+), 33 deletions(-)
>
> diff --git a/console.c b/console.c
> index 57d6eb5..4939a72 100644
> --- a/console.c
> +++ b/console.c
> @@ -1278,35 +1278,40 @@ static DisplaySurface* 
> defaultallocator_create_displaysurface(int width, int hei
>   {
>       DisplaySurface *surface = (DisplaySurface*) 
> qemu_mallocz(sizeof(DisplaySurface));
>
> -    surface->width = width;
> -    surface->height = height;
> -    surface->linesize = width * 4;
> -    surface->pf = qemu_default_pixelformat(32);
> -#ifdef HOST_WORDS_BIGENDIAN
> -    surface->flags = QEMU_ALLOCATED_FLAG | QEMU_BIG_ENDIAN_FLAG;
> -#else
> -    surface->flags = QEMU_ALLOCATED_FLAG;
> -#endif
> -    surface->data = (uint8_t*) qemu_mallocz(surface->linesize * 
> surface->height);
> -
> +    int linesize = width * 4;
> +    surface = qemu_alloc_display(surface, width, height, linesize,
> +                                 qemu_default_pixelformat(32), 0);
>       return surface;
>   }
>
>   static DisplaySurface* defaultallocator_resize_displaysurface(DisplaySurface 
> *surface,
>                                             int width, int height)
>   {
> +    int linesize = width * 4;
> +    surface = qemu_alloc_display(surface, width, height, linesize,
> +                                 qemu_default_pixelformat(32), 0);
> +    return surface;
> +}
> +
> +DisplaySurface*
> +qemu_alloc_display(DisplaySurface *surface, int width, int height,
> +                   int linesize, PixelFormat pf, int newflags)
> +{
> +    void *data;
>       surface->width = width;
>       surface->height = height;
> -    surface->linesize = width * 4;
> -    surface->pf = qemu_default_pixelformat(32);
> -    if (surface->flags&  QEMU_ALLOCATED_FLAG)
> -        surface->data = (uint8_t*) qemu_realloc(surface->data, surface->linesize 
> * surface->height);
> -    else
> -        surface->data = (uint8_t*) qemu_malloc(surface->linesize * 
> surface->height);
> +    surface->linesize = linesize;
> +    surface->pf = pf;
> +    if (surface->flags&  QEMU_ALLOCATED_FLAG) {
> +        data = qemu_realloc(surface->data,
> +                            surface->linesize * surface->height);
> +    } else {
> +        data = qemu_malloc(surface->linesize * surface->height);
> +    }
> +    surface->data = (uint8_t *)data;
> +    surface->flags = newflags | QEMU_ALLOCATED_FLAG;
>   #ifdef HOST_WORDS_BIGENDIAN
> -    surface->flags = QEMU_ALLOCATED_FLAG | QEMU_BIG_ENDIAN_FLAG;
> -#else
> -    surface->flags = QEMU_ALLOCATED_FLAG;
> +    surface->flags |= QEMU_BIG_ENDIAN_FLAG;
>   #endif
>
>       return surface;
> diff --git a/console.h b/console.h
> index f4e4741..dec9a76 100644
> --- a/console.h
> +++ b/console.h
> @@ -189,6 +189,9 @@ void register_displaystate(DisplayState *ds);
>   DisplayState *get_displaystate(void);
>   DisplaySurface* qemu_create_displaysurface_from(int width, int height, int 
> bpp,
>                                                   int linesize, uint8_t *data);
> +DisplaySurface* qemu_alloc_display(DisplaySurface *surface, int width,
> +                                   int height, int linesize,
> +                                   PixelFormat pf, int newflags);

Is it really useful at all to return DisplaySurface?  When I see a 
return value of 'DisplaySurface *' and an alloc in the function name, I 
assume this function allocates a display surface but it's really 
allocating the framebuffer within a display surface.

Regards,

Anthony Liguori