Store width & height & surface in local variables.  Update RAMFBState
with the new values only in case the ramfb_create_display_surface() call
succeeds.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/ramfb.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index 9d41c2ad2868..fbe959147dc9 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -71,20 +71,27 @@ static DisplaySurface *ramfb_create_display_surface(int width, int height,
 static void ramfb_fw_cfg_write(void *dev, off_t offset, size_t len)
 {
     RAMFBState *s = dev;
-    uint32_t fourcc, format;
+    DisplaySurface *surface;
+    uint32_t fourcc, format, width, height;
     hwaddr stride, addr;
 
-    s->width  = be32_to_cpu(s->cfg.width);
-    s->height = be32_to_cpu(s->cfg.height);
-    stride    = be32_to_cpu(s->cfg.stride);
-    fourcc    = be32_to_cpu(s->cfg.fourcc);
-    addr      = be64_to_cpu(s->cfg.addr);
-    format    = qemu_drm_format_to_pixman(fourcc);
+    width  = be32_to_cpu(s->cfg.width);
+    height = be32_to_cpu(s->cfg.height);
+    stride = be32_to_cpu(s->cfg.stride);
+    fourcc = be32_to_cpu(s->cfg.fourcc);
+    addr   = be64_to_cpu(s->cfg.addr);
+    format = qemu_drm_format_to_pixman(fourcc);
 
     fprintf(stderr, "%s: %dx%d @ 0x%" PRIx64 "\n", __func__,
             s->width, s->height, addr);
-    s->ds = ramfb_create_display_surface(s->width, s->height,
-                                         format, stride, addr);
+    surface = ramfb_create_display_surface(width, height,
+                                           format, stride, addr);
+    if (!surface)
+        return;
+
+    s->width = width;
+    s->height = height;
+    s->ds = surface;
 }
 
 void ramfb_display_update(QemuConsole *con, RAMFBState *s)
-- 
2.18.2

On 4/22/20 12:02 PM, Gerd Hoffmann wrote:
> Store width & height & surface in local variables.  Update RAMFBState
> with the new values only in case the ramfb_create_display_surface() call
> succeeds.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/display/ramfb.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
> index 9d41c2ad2868..fbe959147dc9 100644
> --- a/hw/display/ramfb.c
> +++ b/hw/display/ramfb.c
> @@ -71,20 +71,27 @@ static DisplaySurface *ramfb_create_display_surface(int width, int height,
>   static void ramfb_fw_cfg_write(void *dev, off_t offset, size_t len)
>   {
>       RAMFBState *s = dev;
> -    uint32_t fourcc, format;
> +    DisplaySurface *surface;
> +    uint32_t fourcc, format, width, height;
>       hwaddr stride, addr;
>   
> -    s->width  = be32_to_cpu(s->cfg.width);
> -    s->height = be32_to_cpu(s->cfg.height);
> -    stride    = be32_to_cpu(s->cfg.stride);
> -    fourcc    = be32_to_cpu(s->cfg.fourcc);
> -    addr      = be64_to_cpu(s->cfg.addr);
> -    format    = qemu_drm_format_to_pixman(fourcc);
> +    width  = be32_to_cpu(s->cfg.width);
> +    height = be32_to_cpu(s->cfg.height);
> +    stride = be32_to_cpu(s->cfg.stride);
> +    fourcc = be32_to_cpu(s->cfg.fourcc);
> +    addr   = be64_to_cpu(s->cfg.addr);
> +    format = qemu_drm_format_to_pixman(fourcc);
>   
>       fprintf(stderr, "%s: %dx%d @ 0x%" PRIx64 "\n", __func__,
>               s->width, s->height, addr);
> -    s->ds = ramfb_create_display_surface(s->width, s->height,
> -                                         format, stride, addr);
> +    surface = ramfb_create_display_surface(width, height,
> +                                           format, stride, addr);
> +    if (!surface)
> +        return;
> +
> +    s->width = width;
> +    s->height = height;
> +    s->ds = surface;
>   }
>   
>   void ramfb_display_update(QemuConsole *con, RAMFBState *s)
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

On 04/22/20 12:02, Gerd Hoffmann wrote:
> Store width & height & surface in local variables.  Update RAMFBState
> with the new values only in case the ramfb_create_display_surface() call
> succeeds.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/ramfb.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
> index 9d41c2ad2868..fbe959147dc9 100644
> --- a/hw/display/ramfb.c
> +++ b/hw/display/ramfb.c
> @@ -71,20 +71,27 @@ static DisplaySurface *ramfb_create_display_surface(int width, int height,
>  static void ramfb_fw_cfg_write(void *dev, off_t offset, size_t len)
>  {
>      RAMFBState *s = dev;
> -    uint32_t fourcc, format;
> +    DisplaySurface *surface;
> +    uint32_t fourcc, format, width, height;
>      hwaddr stride, addr;
>  
> -    s->width  = be32_to_cpu(s->cfg.width);
> -    s->height = be32_to_cpu(s->cfg.height);
> -    stride    = be32_to_cpu(s->cfg.stride);
> -    fourcc    = be32_to_cpu(s->cfg.fourcc);
> -    addr      = be64_to_cpu(s->cfg.addr);
> -    format    = qemu_drm_format_to_pixman(fourcc);
> +    width  = be32_to_cpu(s->cfg.width);
> +    height = be32_to_cpu(s->cfg.height);
> +    stride = be32_to_cpu(s->cfg.stride);
> +    fourcc = be32_to_cpu(s->cfg.fourcc);
> +    addr   = be64_to_cpu(s->cfg.addr);
> +    format = qemu_drm_format_to_pixman(fourcc);
>  
>      fprintf(stderr, "%s: %dx%d @ 0x%" PRIx64 "\n", __func__,
>              s->width, s->height, addr);
> -    s->ds = ramfb_create_display_surface(s->width, s->height,
> -                                         format, stride, addr);
> +    surface = ramfb_create_display_surface(width, height,
> +                                           format, stride, addr);
> +    if (!surface)
> +        return;

A warning message here, or inside ramfb_create_display_surface(), could
be nice; but I agree it's not required at all.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> +
> +    s->width = width;
> +    s->height = height;
> +    s->ds = surface;
>  }
>  
>  void ramfb_display_update(QemuConsole *con, RAMFBState *s)
>