[Qemu-devel] [PATCH v2 0/3] Remove QEMUFile abuse

[Qemu-devel] [PATCH v2 0/3] Remove QEMUFile abuse

[Juan Quintela]

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(-)

-- 
1.7.6.2

[Qemu-devel] [PATCH 1/3] wavaudio: Use stdio instead of QEMUFile

[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.

Signed-off-by: Juan Quintela <quintela@redhat.com>
CC:  malc <av1474@comtv.ru>

---
 audio/wavaudio.c |   34 ++++++++++++++++++++++++----------
 1 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/audio/wavaudio.c b/audio/wavaudio.c
index aed1817..7b7c081 100644
--- a/audio/wavaudio.c
+++ b/audio/wavaudio.c
@@ -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);
-- 
1.7.6.2

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

[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.

Signed-off-by: Juan Quintela <quintela@redhat.com>
CC: malc <av1474@comtv.ru>

---
 audio/wavcapture.c |   48 ++++++++++++++++++++++++++++++++++--------------
 1 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/audio/wavcapture.c b/audio/wavcapture.c
index c64f0ef..9019995 100644
--- a/audio/wavcapture.c
+++ b/audio/wavcapture.c
@@ -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;
 }
-- 
1.7.6.2

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

[malc]

On Tue, 20 Sep 2011, Juan Quintela wrote:

> 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..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 3/3] ds1225y: Use stdio instead of QEMUFile

[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.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ds1225y.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/ds1225y.c b/hw/ds1225y.c
index 9875c44..6852a61 100644
--- a/hw/ds1225y.c
+++ b/hw/ds1225y.c
@@ -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);

-- 
1.7.6.2