Patchwork [1/7] Consolidate DisplaySurface allocation in qemu_alloc_display()

login
register
mail settings
Submitter Jes Sorensen
Date March 15, 2011, 12:36 p.m.
Message ID <1300192574-32644-2-git-send-email-Jes.Sorensen@redhat.com>
Download mbox | patch
Permalink /patch/86976/
State New
Headers show

Comments

Jes Sorensen - March 15, 2011, 12:36 p.m.
From: Jes Sorensen <Jes.Sorensen@redhat.com>

This removes various code duplication from console.e and sdl.c

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 console.c |   45 +++++++++++++++++++++++++--------------------
 console.h |    3 +++
 ui/sdl.c  |   21 ++++++++-------------
 3 files changed, 36 insertions(+), 33 deletions(-)
Anthony Liguori - March 15, 2011, 2:49 p.m.
On 03/15/2011 07:36 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> This removes various code duplication from console.e and sdl.c
>
> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> ---
>   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
Jes Sorensen - March 15, 2011, 2:53 p.m.
On 03/15/11 15:49, Anthony Liguori wrote:
>> 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.

I am not sure what is better here - if you have a better suggestion for
the name, I am all open. The reason it turned out this way is that there
are already other ways where the DisplaySurface itself can be allocated.

Cheers,
Jes

Patch

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);
 PixelFormat qemu_different_endianness_pixelformat(int bpp);
 PixelFormat qemu_default_pixelformat(int bpp);
 
diff --git a/ui/sdl.c b/ui/sdl.c
index 47ac49c..6c10ea6 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -176,23 +176,18 @@  static DisplaySurface* sdl_create_displaysurface(int width, int height)
 
     surface->width = width;
     surface->height = height;
-    
+
     if (scaling_active) {
+        int linesize;
+        PixelFormat pf;
         if (host_format.BytesPerPixel != 2 && host_format.BytesPerPixel != 4) {
-            surface->linesize = width * 4;
-            surface->pf = qemu_default_pixelformat(32);
+            linesize = width * 4;
+            pf = qemu_default_pixelformat(32);
         } else {
-            surface->linesize = width * host_format.BytesPerPixel;
-            surface->pf = sdl_to_qemu_pixelformat(&host_format);
+            linesize = width * host_format.BytesPerPixel;
+            pf = sdl_to_qemu_pixelformat(&host_format);
         }
-#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);
-
-        return surface;
+        return qemu_alloc_display(surface, width, height, linesize, pf, 0);
     }
 
     if (host_format.BitsPerPixel == 16)