Skip to contentSkip to conversation

Remove QEMUFile abuse

PATCH In review Juan Quintela 2 participants 5 messages Sep 20, 2011

lore
Share & tools
lore
#0 lore raw

v2 cover Remove QEMUFile abuse

Hi

v2: Make malc happy release O;-)
- learn how to use fwrite/fread correctly O:-)
- add errno to all error messages
- make checkpatch happy. Except for
WARNING: space prohibited between function name and open parenthesis '('

Please apply.

v1:

QEMUFile is intended to be used only for migration. Change the other three users to use FILE * operations directly. gcc on Fedora 15 complains about fread/write not checking its return value, so I added checks. But in several places only print an error message (there is no error handly that I can hook into). Notice that this is not worse than it is today.

Later, Juan.

Juan Quintela (3):
wavaudio: Use stdio instead of QEMUFile
wavcapture: Use stdio instead of QEMUFile
ds1225y: Use stdio instead of QEMUFile

 audio/wavaudio.c   |   34 ++++++++++++++++++++++++----------
 audio/wavcapture.c |   48 ++++++++++++++++++++++++++++++++++--------------
 hw/ds1225y.c       |   28 ++++++++++++++++------------
 3 files changed, 74 insertions(+), 36 deletions(-)
Signature · 1 line
1.7.6.2
#1 lore raw

PATCH v2 1/3 wavaudio: Use stdio instead of QEMUFile

QEMUFile * is only intended for migration nowadays. Using it for anything else just adds pain and a layer of buffers for no good reason.

---
 audio/wavaudio.c |   34 ++++++++++++++++++++++++----------
 1 files changed, 24 insertions(+), 10 deletions(-)
audio/wavaudio.c +24 -10
index aed1817..7b7c081 100644
@@ -30,7 +30,7 @@
 
 typedef struct WAVVoiceOut {
     HWVoiceOut hw;
-    QEMUFile *f;
+    FILE *f;
     int64_t old_ticks;
     void *pcm_buf;
     int total_samples;
@@ -76,7 +76,9 @@ static int wav_run_out (HWVoiceOut *hw, int live)
         dst = advance (wav->pcm_buf, rpos << hw->info.shift);
 
         hw->clip (dst, src, convert_samples);
-        qemu_put_buffer (wav->f, dst, convert_samples << hw->info.shift);
+        if (fwrite (dst, convert_samples << hw->info.shift, 1, wav->f) != 1) {
+            dolog ("wav_run_out: fwrite error '%s'\n", strerror(errno));
+        }
 
         rpos = (rpos + convert_samples) % hw->samples;
         samples -= convert_samples;
@@ -152,7 +154,7 @@ static int wav_init_out (HWVoiceOut *hw, struct audsettings *as)
     le_store (hdr + 28, hw->info.freq << (bits16 + stereo), 4);
     le_store (hdr + 32, 1 << (bits16 + stereo), 2);
 
-    wav->f = qemu_fopen (conf.wav_path, "wb");
+    wav->f = fopen (conf.wav_path, "wb");
     if (!wav->f) {
         dolog ("Failed to open wave file `%s'\nReason: %s\n",
                conf.wav_path, strerror (errno));
@@ -161,7 +163,10 @@ static int wav_init_out (HWVoiceOut *hw, struct audsettings *as)
         return -1;
     }
 
-    qemu_put_buffer (wav->f, hdr, sizeof (hdr));
+    if (fwrite (hdr, sizeof (hdr), 1, wav->f) != 1) {
+        dolog ("wav_init_out: fwrite error '%s'\n", strerror(errno));
+        return -1;
+    }
     return 0;
 }
 
@@ -180,13 +185,22 @@ static void wav_fini_out (HWVoiceOut *hw)
     le_store (rlen, rifflen, 4);
     le_store (dlen, datalen, 4);
 
-    qemu_fseek (wav->f, 4, SEEK_SET);
-    qemu_put_buffer (wav->f, rlen, 4);
-
-    qemu_fseek (wav->f, 32, SEEK_CUR);
-    qemu_put_buffer (wav->f, dlen, 4);
+    if (fseek (wav->f, 4, SEEK_SET) == -1) {
+        dolog ("wav_fini_out: fseek error '%s'\n", strerror(errno));
+    }
+    if (fwrite (rlen, 1, 4, wav->f) != 4) {
+        dolog ("wav_fini_out: fwrite error writing rlen '%s'\n",
+               strerror(errno));
+    }
+    if (fseek (wav->f, 32, SEEK_CUR) == -1) {
+        dolog ("wav_fini_out: fseek error '%s'\n", strerror(errno));
+    }
+    if (fwrite (dlen, 1, 4, wav->f) != 4) {
+        dolog ("wav_fini_out: fwrite error writing dlen '%s'\n",
+               strerror(errno));
+    }
 
-    qemu_fclose (wav->f);
+    fclose (wav->f);
     wav->f = NULL;
 
     g_free (wav->pcm_buf);
Signature · 1 line
1.7.6.2
#2 lore raw

PATCH v2 2/3 wavcapture: Use stdio instead of QEMUFile

In reply to Juan Quintela

QEMUFile * is only intended for migration nowadays. Using it for anything else just adds pain and a layer of buffers for no good reason.

---
 audio/wavcapture.c |   48 ++++++++++++++++++++++++++++++++++--------------
 1 files changed, 34 insertions(+), 14 deletions(-)
audio/wavcapture.c +34 -14
index c64f0ef..9019995 100644
@@ -3,7 +3,7 @@
 #include "audio.h"
 
 typedef struct {
-    QEMUFile *f;
+    FILE *f;
     int bytes;
     char *path;
     int freq;
@@ -40,12 +40,22 @@ static void wav_destroy (void *opaque)
         le_store (rlen, rifflen, 4);
         le_store (dlen, datalen, 4);
 
-        qemu_fseek (wav->f, 4, SEEK_SET);
-        qemu_put_buffer (wav->f, rlen, 4);
-
-        qemu_fseek (wav->f, 32, SEEK_CUR);
-        qemu_put_buffer (wav->f, dlen, 4);
-        qemu_fclose (wav->f);
+        fseek (wav->f, 4, SEEK_SET);
+        if (fseek (wav->f, 4, SEEK_SET) == -1) {
+            printf ("wav_destroy: fseek error %s\n", strerror(errno));
+        }
+        if (fwrite (rlen, 1, 4, wav->f) != 4) {
+            printf ("wav_destroy: fwrite error writing rlen '%s'\n",
+                    strerror(errno));
+        }
+        if (fseek (wav->f, 32, SEEK_CUR) == -1) {
+            printf ("wav_destroy: fseek error '%s'\n", strerror(errno));
+        }
+        if (fwrite (dlen, 1, 4, wav->f) != 4) {
+            printf ("wav_destroy: fwrite error writing dlen '%s'\n",
+                    strerror(errno));
+        }
+        fclose (wav->f);
     }
 
     g_free (wav->path);
@@ -55,7 +65,10 @@ static void wav_capture (void *opaque, void *buf, int size)
 {
     WAVState *wav = opaque;
 
-    qemu_put_buffer (wav->f, buf, size);
+    if (fwrite (buf, size, 1, wav->f) != 1) {
+        printf ("wav_capture: fwrite error '%s'\n",
+                strerror(errno));
+    }
     wav->bytes += size;
 }
 
@@ -130,7 +143,7 @@ int wav_start_capture (CaptureState *s, const char *path, int freq,
     le_store (hdr + 28, freq << shift, 4);
     le_store (hdr + 32, 1 << shift, 2);
 
-    wav->f = qemu_fopen (path, "wb");
+    wav->f = fopen (path, "wb");
     if (!wav->f) {
         monitor_printf(mon, "Failed to open wave file `%s'\nReason: %s\n",
                        path, strerror (errno));
@@ -143,19 +156,26 @@ int wav_start_capture (CaptureState *s, const char *path, int freq,
     wav->nchannels = nchannels;
     wav->freq = freq;
 
-    qemu_put_buffer (wav->f, hdr, sizeof (hdr));
+    if (fwrite (hdr, sizeof (hdr), 1, wav->f) != 1) {
+        monitor_printf (mon, "Failed to write header '%s'\n",
+                        strerror(errno));
+        goto error_free;
+    }
 
     cap = AUD_add_capture (&as, &ops, wav);
     if (!cap) {
         monitor_printf(mon, "Failed to add audio capture\n");
-        g_free (wav->path);
-        qemu_fclose (wav->f);
-        g_free (wav);
-        return -1;
+        goto error_free;
     }
 
     wav->cap = cap;
     s->opaque = wav;
     s->ops = wav_capture_ops;
     return 0;
+
+error_free:
+    g_free (wav->path);
+    fclose (wav->f);
+    g_free (wav);
+    return -1;
 }
Signature · 1 line
1.7.6.2
#3 lore raw

Re: [Qemu-devel] [PATCH 2/3] wavcapture: Use stdio instead of QEMUFile

Quoted 3 lines · Juan Quintela

QEMUFile * is only intended for migration nowadays. Using it for anything else just adds pain and a layer of buffers for no good reason.

I've commited two wav patches with some stylistic and changes.

[..snip..]

Signature · 1 line
mailto:[email protected]
#4 lore raw

PATCH v2 3/3 ds1225y: Use stdio instead of QEMUFile

In reply to Juan Quintela

QEMUFile * is only intended for migration nowadays. Using it for anything else just adds pain and a layer of buffers for no good reason.

---
 hw/ds1225y.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)
hw/ds1225y.c +16 -12
index 9875c44..6852a61 100644
@@ -29,7 +29,7 @@ typedef struct {
     DeviceState qdev;
     uint32_t chip_size;
     char *filename;
-    QEMUFile *file;
+    FILE *file;
     uint8_t *contents;
 } NvRamState;
 
@@ -70,9 +70,9 @@ static void nvram_writeb (void *opaque, target_phys_addr_t addr, uint32_t val)
 
     s->contents[addr] = val;
     if (s->file) {
-        qemu_fseek(s->file, addr, SEEK_SET);
-        qemu_put_byte(s->file, (int)val);
-        qemu_fflush(s->file);
+        fseek(s->file, addr, SEEK_SET);
+        fputc(val, s->file);
+        fflush(s->file);
     }
 }
 
@@ -108,15 +108,17 @@ static int nvram_post_load(void *opaque, int version_id)
 
     /* Close file, as filename may has changed in load/store process */
     if (s->file) {
-        qemu_fclose(s->file);
+        fclose(s->file);
     }
 
     /* Write back nvram contents */
-    s->file = qemu_fopen(s->filename, "wb");
+    s->file = fopen(s->filename, "wb");
     if (s->file) {
         /* Write back contents, as 'wb' mode cleaned the file */
-        qemu_put_buffer(s->file, s->contents, s->chip_size);
-        qemu_fflush(s->file);
+        if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
+            printf("nvram_post_load: short write\n");
+        }
+        fflush(s->file);
     }
 
     return 0;
@@ -143,7 +145,7 @@ typedef struct {
 static int nvram_sysbus_initfn(SysBusDevice *dev)
 {
     NvRamState *s = &FROM_SYSBUS(SysBusNvRamState, dev)->nvram;
-    QEMUFile *file;
+    FILE *file;
     int s_io;
 
     s->contents = g_malloc0(s->chip_size);
@@ -153,11 +155,13 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
     sysbus_init_mmio(dev, s->chip_size, s_io);
 
     /* Read current file */
-    file = qemu_fopen(s->filename, "rb");
+    file = fopen(s->filename, "rb");
     if (file) {
         /* Read nvram contents */
-        qemu_get_buffer(file, s->contents, s->chip_size);
-        qemu_fclose(file);
+        if (fread(s->contents, s->chip_size, 1, file) != 1) {
+            printf("nvram_sysbus_initfn: short read\n");
+        }
+        fclose(file);
     }
     nvram_post_load(s, 0);
 
Signature · 1 line
1.7.6.2