* [Qemu-devel] [PATCH 1/9] error: add error_setg()
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
@ 2012-08-29 19:52 ` Luiz Capitulino
2012-08-29 19:52 ` [Qemu-devel] [PATCH 2/9] console: vga_hw_screen_dump_ptr: take Error argument Luiz Capitulino
` (7 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:52 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
error.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/error.h b/error.h
index 96fc203..da7fed3 100644
--- a/error.h
+++ b/error.h
@@ -30,6 +30,12 @@ typedef struct Error Error;
void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4);
/**
+ * Same as error_set(), but sets a generic error
+ */
+#define error_setg(err, fmt, ...) \
+ error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
+
+/**
* Returns true if an indirect pointer to an error is pointing to a valid
* error object.
*/
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 2/9] console: vga_hw_screen_dump_ptr: take Error argument
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
2012-08-29 19:52 ` [Qemu-devel] [PATCH 1/9] error: add error_setg() Luiz Capitulino
@ 2012-08-29 19:52 ` Luiz Capitulino
2012-08-29 19:53 ` [Qemu-devel] [PATCH 3/9] qapi: convert screendump Luiz Capitulino
` (6 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:52 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
All devices that register a screen dump callback via
graphic_console_init() are updated.
The new argument is not used in this commit. Error handling will
be added to each device individually later.
This change is a preparation to convert the screendump command
to the QAPI.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
console.c | 2 +-
console.h | 4 +++-
hw/blizzard.c | 2 +-
hw/g364fb.c | 3 ++-
hw/omap_lcdc.c | 3 ++-
hw/qxl.c | 5 +++--
hw/tcx.c | 12 ++++++++----
hw/vga.c | 6 ++++--
hw/vmware_vga.c | 5 +++--
9 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/console.c b/console.c
index 4525cc7..bd501aa 100644
--- a/console.c
+++ b/console.c
@@ -190,7 +190,7 @@ void vga_hw_screen_dump(const char *filename)
console_select(0);
}
if (consoles[0] && consoles[0]->hw_screen_dump) {
- consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch);
+ consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch, NULL);
} else {
error_report("screen dump not implemented");
}
diff --git a/console.h b/console.h
index 4334db5..9caeb43 100644
--- a/console.h
+++ b/console.h
@@ -6,6 +6,7 @@
#include "notify.h"
#include "monitor.h"
#include "trace.h"
+#include "error.h"
/* keyboard/mouse support */
@@ -343,7 +344,8 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
typedef void (*vga_hw_update_ptr)(void *);
typedef void (*vga_hw_invalidate_ptr)(void *);
-typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch);
+typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch,
+ Error **errp);
typedef void (*vga_hw_text_update_ptr)(void *, console_ch_t *);
DisplayState *graphic_console_init(vga_hw_update_ptr update,
diff --git a/hw/blizzard.c b/hw/blizzard.c
index 29074c4..a2b9053 100644
--- a/hw/blizzard.c
+++ b/hw/blizzard.c
@@ -933,7 +933,7 @@ static void blizzard_update_display(void *opaque)
}
static void blizzard_screen_dump(void *opaque, const char *filename,
- bool cswitch)
+ bool cswitch, Error **errp)
{
BlizzardState *s = (BlizzardState *) opaque;
diff --git a/hw/g364fb.c b/hw/g364fb.c
index 3a0b68f..498154b 100644
--- a/hw/g364fb.c
+++ b/hw/g364fb.c
@@ -289,7 +289,8 @@ static void g364fb_reset(G364State *s)
g364fb_invalidate_display(s);
}
-static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
G364State *s = opaque;
int y, x;
diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
index 4a08e9d..39b78cd 100644
--- a/hw/omap_lcdc.c
+++ b/hw/omap_lcdc.c
@@ -264,7 +264,8 @@ static int ppm_save(const char *filename, uint8_t *data,
return 0;
}
-static void omap_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void omap_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
struct omap_lcd_panel_s *omap_lcd = opaque;
diff --git a/hw/qxl.c b/hw/qxl.c
index c2dd3b4..46a929d 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1595,7 +1595,8 @@ static void qxl_hw_invalidate(void *opaque)
vga->invalidate(vga);
}
-static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
PCIQXLDevice *qxl = opaque;
VGACommonState *vga = &qxl->vga;
@@ -1607,7 +1608,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
ppm_save(filename, qxl->ssd.ds->surface);
break;
case QXL_MODE_VGA:
- vga->screen_dump(vga, filename, cswitch);
+ vga->screen_dump(vga, filename, cswitch, errp);
break;
default:
break;
diff --git a/hw/tcx.c b/hw/tcx.c
index ac7dcb4..74a7085 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -56,8 +56,10 @@ typedef struct TCXState {
uint8_t dac_index, dac_state;
} TCXState;
-static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch);
-static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch);
+static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp);
+static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp);
static void tcx_set_dirty(TCXState *s)
{
@@ -574,7 +576,8 @@ static int tcx_init1(SysBusDevice *dev)
return 0;
}
-static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
TCXState *s = opaque;
FILE *f;
@@ -601,7 +604,8 @@ static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
return;
}
-static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
TCXState *s = opaque;
FILE *f;
diff --git a/hw/vga.c b/hw/vga.c
index f82ced8..dd703cf 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -166,7 +166,8 @@ static uint32_t expand4[256];
static uint16_t expand2[256];
static uint8_t expand4to8[16];
-static void vga_screen_dump(void *opaque, const char *filename, bool cswitch);
+static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp);
static void vga_update_memory_access(VGACommonState *s)
{
@@ -2435,7 +2436,8 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
/* save the vga display in a PPM image even if no display is
available */
-static void vga_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
VGACommonState *s = opaque;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index f5e4f44..29750e1 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1003,11 +1003,12 @@ static void vmsvga_invalidate_display(void *opaque)
/* save the vga display in a PPM image even if no display is
available */
-static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
struct vmsvga_state_s *s = opaque;
if (!s->enable) {
- s->vga.screen_dump(&s->vga, filename, cswitch);
+ s->vga.screen_dump(&s->vga, filename, cswitch, errp);
return;
}
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 3/9] qapi: convert screendump
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
2012-08-29 19:52 ` [Qemu-devel] [PATCH 1/9] error: add error_setg() Luiz Capitulino
2012-08-29 19:52 ` [Qemu-devel] [PATCH 2/9] console: vga_hw_screen_dump_ptr: take Error argument Luiz Capitulino
@ 2012-08-29 19:53 ` Luiz Capitulino
2012-08-29 19:53 ` [Qemu-devel] [PATCH 4/9] vga: ppm_save(): add error handling Luiz Capitulino
` (5 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:53 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
Next commits will update devices to propagate errors.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
console.c | 7 ++++---
console.h | 1 -
hmp-commands.hx | 3 +--
hmp.c | 9 +++++++++
hmp.h | 1 +
monitor.c | 6 ------
qapi-schema.json | 13 +++++++++++++
qmp-commands.hx | 5 +----
8 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/console.c b/console.c
index bd501aa..9ab95d8 100644
--- a/console.c
+++ b/console.c
@@ -24,6 +24,7 @@
#include "qemu-common.h"
#include "console.h"
#include "qemu-timer.h"
+#include "qmp-commands.h"
//#define DEBUG_CONSOLE
#define DEFAULT_BACKSCROLL 512
@@ -176,7 +177,7 @@ void vga_hw_invalidate(void)
active_console->hw_invalidate(active_console->hw);
}
-void vga_hw_screen_dump(const char *filename)
+void qmp_screendump(const char *filename, Error **errp)
{
TextConsole *previous_active_console;
bool cswitch;
@@ -190,9 +191,9 @@ void vga_hw_screen_dump(const char *filename)
console_select(0);
}
if (consoles[0] && consoles[0]->hw_screen_dump) {
- consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch, NULL);
+ consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch, errp);
} else {
- error_report("screen dump not implemented");
+ error_setg(errp, "device doesn't support screendump\n");
}
if (cswitch) {
diff --git a/console.h b/console.h
index 9caeb43..d72c546 100644
--- a/console.h
+++ b/console.h
@@ -356,7 +356,6 @@ DisplayState *graphic_console_init(vga_hw_update_ptr update,
void vga_hw_update(void);
void vga_hw_invalidate(void);
-void vga_hw_screen_dump(const char *filename);
void vga_hw_text_update(console_ch_t *chardata);
int is_graphic_console(void);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f6104b0..cb09e9d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -194,8 +194,7 @@ ETEXI
.args_type = "filename:F",
.params = "filename",
.help = "save screen into PPM image 'filename'",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_screen_dump,
+ .mhandler.cmd = hmp_screen_dump,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 81c8acb..ad1dbe7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1102,3 +1102,12 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
qmp_closefd(fdname, &errp);
hmp_handle_error(mon, &errp);
}
+
+void hmp_screen_dump(Monitor *mon, const QDict *qdict)
+{
+ const char *filename = qdict_get_str(qdict, "filename");
+ Error *err = NULL;
+
+ qmp_screendump(filename, &err);
+ hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 7dd93bf..1278abd 100644
--- a/hmp.h
+++ b/hmp.h
@@ -71,5 +71,6 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict);
void hmp_netdev_del(Monitor *mon, const QDict *qdict);
void hmp_getfd(Monitor *mon, const QDict *qdict);
void hmp_closefd(Monitor *mon, const QDict *qdict);
+void hmp_screen_dump(Monitor *mon, const QDict *qdict);
#endif
diff --git a/monitor.c b/monitor.c
index b17b1bb..d285ef3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1016,12 +1016,6 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
return -1;
}
-static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
- vga_hw_screen_dump(qdict_get_str(qdict, "filename"));
- return 0;
-}
-
static void do_logfile(Monitor *mon, const QDict *qdict)
{
cpu_set_log_filename(qdict_get_str(qdict, "filename"));
diff --git a/qapi-schema.json b/qapi-schema.json
index bd8ad74..99192ed 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2493,3 +2493,16 @@
# Since: 1.2.0
##
{ 'command': 'query-target', 'returns': 'TargetInfo' }
+
+##
+# @screendump:
+#
+# Write a PPM of the VGA screen to a file.
+#
+# @filename: the path of a new PPM file to store the image
+#
+# Returns: Nothing on success
+#
+# Since: 0.14.0
+##
+{ 'command': 'screendump', 'data': {'filename': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3745a21..0edc923 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -146,10 +146,7 @@ EQMP
{
.name = "screendump",
.args_type = "filename:F",
- .params = "filename",
- .help = "save screen into PPM image 'filename'",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_screen_dump,
+ .mhandler.cmd_new = qmp_marshal_input_screendump,
},
SQMP
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 4/9] vga: ppm_save(): add error handling
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
` (2 preceding siblings ...)
2012-08-29 19:53 ` [Qemu-devel] [PATCH 3/9] qapi: convert screendump Luiz Capitulino
@ 2012-08-29 19:53 ` Luiz Capitulino
2012-08-29 19:53 ` [Qemu-devel] [PATCH 5/9] omap_lcdc: rename ppm_save() to omap_ppm_save() Luiz Capitulino
` (4 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:53 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
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, 30 insertions(+), 11 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 46a929d..bae5758 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1605,7 +1605,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 dd703cf..80299ea 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2390,7 +2390,7 @@ 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;
@@ -2402,10 +2402,16 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
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);
+ if (!f) {
+ error_setg(errp, "failed to open file '%s': %s", filename,
+ strerror(errno));
+ return;
+ }
+ ret = fprintf(f, "P6\n%d %d\n%d\n", ds->width, ds->height, 255);
+ if (ret < 0) {
+ linebuf = NULL;
+ goto write_err;
+ }
linebuf = g_malloc(ds->width * 3);
d1 = ds->data;
for(y = 0; y < ds->height; y++) {
@@ -2426,12 +2432,24 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
d += ds->pf.bytes_per_pixel;
}
d1 += ds->linesize;
+ clearerr(f);
ret = fwrite(linebuf, 1, pbuf - linebuf, f);
(void)ret;
+ if (ferror(f)) {
+ goto write_err;
+ }
}
+
+out:
g_free(linebuf);
fclose(f);
- return 0;
+ return;
+
+write_err:
+ error_setg(errp, "failed to write to file '%s': %s", filename,
+ strerror(errno));
+ unlink(filename);
+ goto out;
}
/* save the vga display in a PPM image even if no display is
@@ -2445,5 +2463,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 8938093..330a32f 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
@@ -204,7 +205,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 29750e1..b68e883 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.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 5/9] omap_lcdc: rename ppm_save() to omap_ppm_save()
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
` (3 preceding siblings ...)
2012-08-29 19:53 ` [Qemu-devel] [PATCH 4/9] vga: ppm_save(): add error handling Luiz Capitulino
@ 2012-08-29 19:53 ` Luiz Capitulino
2012-08-29 21:26 ` Peter Maydell
2012-08-29 19:53 ` [Qemu-devel] [PATCH 6/9] omap_lcdc: omap_ppm_save(): add error handling Luiz Capitulino
` (3 subsequent siblings)
8 siblings, 1 reply; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:53 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
Avoids confusion with the global ppm_save() defined in hw/vga.c.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/omap_lcdc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
index 39b78cd..3d6328f 100644
--- a/hw/omap_lcdc.c
+++ b/hw/omap_lcdc.c
@@ -224,8 +224,8 @@ static void omap_update_display(void *opaque)
omap_lcd->invalidate = 0;
}
-static int ppm_save(const char *filename, uint8_t *data,
- int w, int h, int linesize)
+static int omap_ppm_save(const char *filename, uint8_t *data,
+ int w, int h, int linesize)
{
FILE *f;
uint8_t *d, *d1;
@@ -271,9 +271,9 @@ static void omap_screen_dump(void *opaque, const char *filename, bool cswitch,
omap_update_display(opaque);
if (omap_lcd && ds_get_data(omap_lcd->state))
- ppm_save(filename, ds_get_data(omap_lcd->state),
- omap_lcd->width, omap_lcd->height,
- ds_get_linesize(omap_lcd->state));
+ omap_ppm_save(filename, ds_get_data(omap_lcd->state),
+ omap_lcd->width, omap_lcd->height,
+ ds_get_linesize(omap_lcd->state));
}
static void omap_invalidate_display(void *opaque) {
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 6/9] omap_lcdc: omap_ppm_save(): add error handling
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
` (4 preceding siblings ...)
2012-08-29 19:53 ` [Qemu-devel] [PATCH 5/9] omap_lcdc: rename ppm_save() to omap_ppm_save() Luiz Capitulino
@ 2012-08-29 19:53 ` Luiz Capitulino
2012-08-29 21:28 ` Peter Maydell
2012-08-29 19:53 ` [Qemu-devel] [PATCH 7/9] g364fb: g364fb_screen_dump(): " Luiz Capitulino
` (2 subsequent siblings)
8 siblings, 1 reply; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:53 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/omap_lcdc.c | 59 ++++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 14 deletions(-)
diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
index 3d6328f..e2ba108 100644
--- a/hw/omap_lcdc.c
+++ b/hw/omap_lcdc.c
@@ -224,18 +224,24 @@ static void omap_update_display(void *opaque)
omap_lcd->invalidate = 0;
}
-static int omap_ppm_save(const char *filename, uint8_t *data,
- int w, int h, int linesize)
+static void omap_ppm_save(const char *filename, uint8_t *data,
+ int w, int h, int linesize, Error **errp)
{
FILE *f;
uint8_t *d, *d1;
unsigned int v;
- int y, x, bpp;
+ int ret, y, x, bpp;
f = fopen(filename, "wb");
- if (!f)
- return -1;
- fprintf(f, "P6\n%d %d\n%d\n", w, h, 255);
+ if (!f) {
+ error_setg(errp, "failed to open file '%s': %s", filename,
+ strerror(errno));
+ return;
+ }
+ ret = fprintf(f, "P6\n%d %d\n%d\n", w, h, 255);
+ if (ret < 0) {
+ goto write_err;
+ }
d1 = data;
bpp = linesize / w;
for (y = 0; y < h; y ++) {
@@ -244,24 +250,49 @@ static int omap_ppm_save(const char *filename, uint8_t *data,
v = *(uint32_t *) d;
switch (bpp) {
case 2:
- fputc((v >> 8) & 0xf8, f);
- fputc((v >> 3) & 0xfc, f);
- fputc((v << 3) & 0xf8, f);
+ ret = fputc((v >> 8) & 0xf8, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc((v >> 3) & 0xfc, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc((v << 3) & 0xf8, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
break;
case 3:
case 4:
default:
- fputc((v >> 16) & 0xff, f);
- fputc((v >> 8) & 0xff, f);
- fputc((v) & 0xff, f);
+ ret = fputc((v >> 16) & 0xff, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc((v >> 8) & 0xff, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc((v) & 0xff, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
break;
}
d += bpp;
}
d1 += linesize;
}
+out:
fclose(f);
- return 0;
+ return;
+
+write_err:
+ error_setg(errp, "failed to write to file '%s': %s", filename,
+ strerror(errno));
+ unlink(filename);
+ goto out;
}
static void omap_screen_dump(void *opaque, const char *filename, bool cswitch,
@@ -273,7 +304,7 @@ static void omap_screen_dump(void *opaque, const char *filename, bool cswitch,
if (omap_lcd && ds_get_data(omap_lcd->state))
omap_ppm_save(filename, ds_get_data(omap_lcd->state),
omap_lcd->width, omap_lcd->height,
- ds_get_linesize(omap_lcd->state));
+ ds_get_linesize(omap_lcd->state), errp);
}
static void omap_invalidate_display(void *opaque) {
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH 6/9] omap_lcdc: omap_ppm_save(): add error handling
2012-08-29 19:53 ` [Qemu-devel] [PATCH 6/9] omap_lcdc: omap_ppm_save(): add error handling Luiz Capitulino
@ 2012-08-29 21:28 ` Peter Maydell
2012-08-30 11:38 ` Luiz Capitulino
0 siblings, 1 reply; 13+ messages in thread
From: Peter Maydell @ 2012-08-29 21:28 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: alevy, qemu-devel
On 29 August 2012 20:53, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> hw/omap_lcdc.c | 59 ++++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 45 insertions(+), 14 deletions(-)
>
> diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
> index 3d6328f..e2ba108 100644
> --- a/hw/omap_lcdc.c
> +++ b/hw/omap_lcdc.c
> @@ -224,18 +224,24 @@ static void omap_update_display(void *opaque)
> omap_lcd->invalidate = 0;
> }
>
> -static int omap_ppm_save(const char *filename, uint8_t *data,
> - int w, int h, int linesize)
> +static void omap_ppm_save(const char *filename, uint8_t *data,
> + int w, int h, int linesize, Error **errp)
> {
> FILE *f;
> uint8_t *d, *d1;
> unsigned int v;
> - int y, x, bpp;
> + int ret, y, x, bpp;
>
> f = fopen(filename, "wb");
> - if (!f)
> - return -1;
> - fprintf(f, "P6\n%d %d\n%d\n", w, h, 255);
> + if (!f) {
> + error_setg(errp, "failed to open file '%s': %s", filename,
> + strerror(errno));
> + return;
> + }
> + ret = fprintf(f, "P6\n%d %d\n%d\n", w, h, 255);
> + if (ret < 0) {
> + goto write_err;
> + }
We don't use 'ret' in write_err, so why not just
if (fprintf(f....) < 0) {
goto write_err;
}
here and similarly below and drop the variable altogether?
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH 6/9] omap_lcdc: omap_ppm_save(): add error handling
2012-08-29 21:28 ` Peter Maydell
@ 2012-08-30 11:38 ` Luiz Capitulino
0 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-30 11:38 UTC (permalink / raw)
To: Peter Maydell; +Cc: alevy, qemu-devel
On Wed, 29 Aug 2012 22:28:38 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:
> On 29 August 2012 20:53, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> > hw/omap_lcdc.c | 59 ++++++++++++++++++++++++++++++++++++++++++++--------------
> > 1 file changed, 45 insertions(+), 14 deletions(-)
> >
> > diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
> > index 3d6328f..e2ba108 100644
> > --- a/hw/omap_lcdc.c
> > +++ b/hw/omap_lcdc.c
> > @@ -224,18 +224,24 @@ static void omap_update_display(void *opaque)
> > omap_lcd->invalidate = 0;
> > }
> >
> > -static int omap_ppm_save(const char *filename, uint8_t *data,
> > - int w, int h, int linesize)
> > +static void omap_ppm_save(const char *filename, uint8_t *data,
> > + int w, int h, int linesize, Error **errp)
> > {
> > FILE *f;
> > uint8_t *d, *d1;
> > unsigned int v;
> > - int y, x, bpp;
> > + int ret, y, x, bpp;
> >
> > f = fopen(filename, "wb");
> > - if (!f)
> > - return -1;
> > - fprintf(f, "P6\n%d %d\n%d\n", w, h, 255);
> > + if (!f) {
> > + error_setg(errp, "failed to open file '%s': %s", filename,
> > + strerror(errno));
> > + return;
> > + }
> > + ret = fprintf(f, "P6\n%d %d\n%d\n", w, h, 255);
> > + if (ret < 0) {
> > + goto write_err;
> > + }
>
> We don't use 'ret' in write_err, so why not just
> if (fprintf(f....) < 0) {
> goto write_err;
> }
>
> here and similarly below and drop the variable altogether?
For clarity. This is probably a matter of taste, but I much more prefer
separate statements (vs. saving 4 bytes during the function call).
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 7/9] g364fb: g364fb_screen_dump(): add error handling
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
` (5 preceding siblings ...)
2012-08-29 19:53 ` [Qemu-devel] [PATCH 6/9] omap_lcdc: omap_ppm_save(): add error handling Luiz Capitulino
@ 2012-08-29 19:53 ` Luiz Capitulino
2012-08-29 19:53 ` [Qemu-devel] [PATCH 8/9] tcx: tcx24_screen_dump(): " Luiz Capitulino
2012-08-29 19:53 ` [Qemu-devel] [PATCH 9/9] tcx: tcx_screen_dump(): " Luiz Capitulino
8 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:53 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/g364fb.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 12 deletions(-)
diff --git a/hw/g364fb.c b/hw/g364fb.c
index 498154b..059e622 100644
--- a/hw/g364fb.c
+++ b/hw/g364fb.c
@@ -293,7 +293,7 @@ static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch,
Error **errp)
{
G364State *s = opaque;
- int y, x;
+ int ret, y, x;
uint8_t index;
uint8_t *data_buffer;
FILE *f;
@@ -301,35 +301,63 @@ static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch,
qemu_flush_coalesced_mmio_buffer();
if (s->depth != 8) {
- error_report("g364: unknown guest depth %d", s->depth);
+ error_setg(errp, "g364: unknown guest depth %d", s->depth);
return;
}
f = fopen(filename, "wb");
- if (!f)
+ if (!f) {
+ error_setg(errp, "failed to open file '%s': %s", filename,
+ strerror(errno));
return;
+ }
if (s->ctla & CTLA_FORCE_BLANK) {
/* blank screen */
- fprintf(f, "P4\n%d %d\n",
- s->width, s->height);
+ ret = fprintf(f, "P4\n%d %d\n", s->width, s->height);
+ if (ret < 0) {
+ goto write_err;
+ }
for (y = 0; y < s->height; y++)
- for (x = 0; x < s->width; x++)
- fputc(0, f);
+ for (x = 0; x < s->width; x++) {
+ ret = fputc(0, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ }
} else {
data_buffer = s->vram + s->top_of_screen;
- fprintf(f, "P6\n%d %d\n%d\n",
- s->width, s->height, 255);
+ ret = fprintf(f, "P6\n%d %d\n%d\n", s->width, s->height, 255);
+ if (ret < 0) {
+ goto write_err;
+ }
for (y = 0; y < s->height; y++)
for (x = 0; x < s->width; x++, data_buffer++) {
index = *data_buffer;
- fputc(s->color_palette[index][0], f);
- fputc(s->color_palette[index][1], f);
- fputc(s->color_palette[index][2], f);
+ ret = fputc(s->color_palette[index][0], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc(s->color_palette[index][1], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc(s->color_palette[index][2], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
}
}
+out:
fclose(f);
+ return;
+
+write_err:
+ error_setg(errp, "failed to write to file '%s': %s", filename,
+ strerror(errno));
+ unlink(filename);
+ goto out;
}
/* called for accesses to io ports */
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 8/9] tcx: tcx24_screen_dump(): add error handling
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
` (6 preceding siblings ...)
2012-08-29 19:53 ` [Qemu-devel] [PATCH 7/9] g364fb: g364fb_screen_dump(): " Luiz Capitulino
@ 2012-08-29 19:53 ` Luiz Capitulino
2012-08-29 19:53 ` [Qemu-devel] [PATCH 9/9] tcx: tcx_screen_dump(): " Luiz Capitulino
8 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:53 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/tcx.c | 50 +++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 41 insertions(+), 9 deletions(-)
diff --git a/hw/tcx.c b/hw/tcx.c
index 74a7085..428649e 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -611,12 +611,18 @@ static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
FILE *f;
uint8_t *d, *d1, v;
uint32_t *s24, *cptr, dval;
- int y, x;
+ int ret, y, x;
f = fopen(filename, "wb");
- if (!f)
+ if (!f) {
+ error_setg(errp, "failed to open file '%s': %s", filename,
+ strerror(errno));
return;
- fprintf(f, "P6\n%d %d\n%d\n", s->width, s->height, 255);
+ }
+ ret = fprintf(f, "P6\n%d %d\n%d\n", s->width, s->height, 255);
+ if (ret < 0) {
+ goto write_err;
+ }
d1 = s->vram;
s24 = s->vram24;
cptr = s->cplane;
@@ -625,20 +631,46 @@ static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
for(x = 0; x < s->width; x++, d++, s24++) {
if ((*cptr++ & 0xff000000) == 0x03000000) { // 24-bit direct
dval = *s24 & 0x00ffffff;
- fputc((dval >> 16) & 0xff, f);
- fputc((dval >> 8) & 0xff, f);
- fputc(dval & 0xff, f);
+ ret = fputc((dval >> 16) & 0xff, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc((dval >> 8) & 0xff, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc(dval & 0xff, f);
+ if (ret == EOF) {
+ goto write_err;
+ }
} else {
v = *d;
- fputc(s->r[v], f);
- fputc(s->g[v], f);
- fputc(s->b[v], f);
+ ret = fputc(s->r[v], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc(s->g[v], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc(s->b[v], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
}
}
d1 += MAXX;
}
+
+out:
fclose(f);
return;
+
+write_err:
+ error_setg(errp, "failed to write to file '%s': %s", filename,
+ strerror(errno));
+ unlink(filename);
+ goto out;
}
static Property tcx_properties[] = {
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 9/9] tcx: tcx_screen_dump(): add error handling
2012-08-29 19:52 [Qemu-devel] [PATCH qmp-next v3 0/9]: qapi: convert screendump Luiz Capitulino
` (7 preceding siblings ...)
2012-08-29 19:53 ` [Qemu-devel] [PATCH 8/9] tcx: tcx24_screen_dump(): " Luiz Capitulino
@ 2012-08-29 19:53 ` Luiz Capitulino
8 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2012-08-29 19:53 UTC (permalink / raw)
To: qemu-devel; +Cc: alevy
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/tcx.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/hw/tcx.c b/hw/tcx.c
index 428649e..93994d6 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -582,26 +582,49 @@ static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
TCXState *s = opaque;
FILE *f;
uint8_t *d, *d1, v;
- int y, x;
+ int ret, y, x;
f = fopen(filename, "wb");
- if (!f)
+ if (!f) {
+ error_setg(errp, "failed to open file '%s': %s", filename,
+ strerror(errno));
return;
- fprintf(f, "P6\n%d %d\n%d\n", s->width, s->height, 255);
+ }
+ ret = fprintf(f, "P6\n%d %d\n%d\n", s->width, s->height, 255);
+ if (ret < 0) {
+ goto write_err;
+ }
d1 = s->vram;
for(y = 0; y < s->height; y++) {
d = d1;
for(x = 0; x < s->width; x++) {
v = *d;
- fputc(s->r[v], f);
- fputc(s->g[v], f);
- fputc(s->b[v], f);
+ ret = fputc(s->r[v], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc(s->g[v], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
+ ret = fputc(s->b[v], f);
+ if (ret == EOF) {
+ goto write_err;
+ }
d++;
}
d1 += MAXX;
}
+
+out:
fclose(f);
return;
+
+write_err:
+ error_setg(errp, "failed to write to file '%s': %s", filename,
+ strerror(errno));
+ unlink(filename);
+ goto out;
}
static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread