[Qemu-devel] [PATCH 09/14] vga: ppm_save(): add error handling

[Luiz Capitulino]

This is done by using qemu_fopen_err(), qemu_fwrite_err() and handling
errors appropriately (eg. removing the screendump file if the operation
fails).

Adding error handling to ppm_save() has the net effect of automatically
adding error handling for all devices using ppm_save(), but note that
the error is not passed up yet, as vga_hw_screen_dump() still calls
consoles[0]->hw_screen_dump() with errp=NULL.

The error will be propagated up when screendump is converted to the QAPI.
That will be done by a later commit.

Signed-off-by: Luiz Capitulino <address@hidden>
---
 hw/blizzard.c   |  2 +-
 hw/qxl.c        |  2 +-
 hw/vga.c        | 32 +++++++++++++++++++++-----------
 hw/vga_int.h    |  3 ++-
 hw/vmware_vga.c |  2 +-
 5 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/hw/blizzard.c b/hw/blizzard.c
index a2b9053..d1c9d81 100644
--- a/hw/blizzard.c
+++ b/hw/blizzard.c
@@ -939,7 +939,7 @@ static void blizzard_screen_dump(void *opaque, const char 
*filename,
 
     blizzard_update_display(opaque);
     if (s && ds_get_data(s->state))
-        ppm_save(filename, s->state->surface);
+        ppm_save(filename, s->state->surface, errp);
 }
 
 #define DEPTH 8
diff --git a/hw/qxl.c b/hw/qxl.c
index 4dc0b7d..e32e517 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1581,7 +1581,7 @@ static void qxl_hw_screen_dump(void *opaque, const char 
*filename, bool cswitch,
     case QXL_MODE_COMPAT:
     case QXL_MODE_NATIVE:
         qxl_render_update(qxl);
-        ppm_save(filename, qxl->ssd.ds->surface);
+        ppm_save(filename, qxl->ssd.ds->surface, errp);
         break;
     case QXL_MODE_VGA:
         vga->screen_dump(vga, filename, cswitch, errp);
diff --git a/hw/vga.c b/hw/vga.c
index e6b154c..feb808f 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2368,22 +2368,25 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion 
*system_memory)
 /********************************************************/
 /* vga screen dump */
 
-int ppm_save(const char *filename, struct DisplaySurface *ds)
+void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp)
 {
     FILE *f;
     uint8_t *d, *d1;
     uint32_t v;
     int y, x;
     uint8_t r, g, b;
-    int ret;
     char *linebuf, *pbuf;
 
     trace_ppm_save(filename, ds);
-    f = fopen(filename, "wb");
-    if (!f)
-        return -1;
-    fprintf(f, "P6\n%d %d\n%d\n",
-            ds->width, ds->height, 255);
+    f = qemu_fopen_err(filename, "wb", errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+    qemu_fprintf_err(errp, f, "P6\n%d %d\n%d\n", ds->width, ds->height, 255);
+    if (error_is_set(errp)) {
+        linebuf = NULL;
+        goto out;
+    }
     linebuf = g_malloc(ds->width * 3);
     d1 = ds->data;
     for(y = 0; y < ds->height; y++) {
@@ -2404,12 +2407,19 @@ int ppm_save(const char *filename, struct 
DisplaySurface *ds)
             d += ds->pf.bytes_per_pixel;
         }
         d1 += ds->linesize;
-        ret = fwrite(linebuf, 1, pbuf - linebuf, f);
-        (void)ret;
+        qemu_fwrite_err(linebuf, 1, pbuf - linebuf, f, errp);
+        if (error_is_set(errp)) {
+            break;
+        }
     }
+
+out:
     g_free(linebuf);
     fclose(f);
-    return 0;
+
+    if (error_is_set(errp)) {
+        unlink(filename);
+    }
 }
 
 /* save the vga display in a PPM image even if no display is
@@ -2423,5 +2433,5 @@ static void vga_screen_dump(void *opaque, const char 
*filename, bool cswitch,
         vga_invalidate_display(s);
     }
     vga_hw_update();
-    ppm_save(filename, s->ds->surface);
+    ppm_save(filename, s->ds->surface, errp);
 }
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 7685b2b..7234cdf 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -23,6 +23,7 @@
  */
 
 #include <hw/hw.h>
+#include "error.h"
 #include "memory.h"
 
 #define ST01_V_RETRACE      0x08
@@ -200,7 +201,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t 
val);
 uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr);
 void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val);
 void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
-int ppm_save(const char *filename, struct DisplaySurface *ds);
+void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp);
 
 int vga_ioport_invalid(VGACommonState *s, uint32_t addr);
 void vga_init_vbe(VGACommonState *s, MemoryRegion *address_space);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 171b1d9..4e68ea4 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1015,7 +1015,7 @@ static void vmsvga_screen_dump(void *opaque, const char 
*filename, bool cswitch,
     if (s->depth == 32) {
         DisplaySurface *ds = qemu_create_displaysurface_from(s->width,
                 s->height, 32, ds_get_linesize(s->vga.ds), s->vga.vram_ptr);
-        ppm_save(filename, ds);
+        ppm_save(filename, ds, errp);
         g_free(ds);
     }
 }
-- 
1.7.10.2.565.gbd578b5