Remove QEMUFile abuse
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
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
@@ -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
PATCH v2 2/3 wavcapture: Use stdio instead of QEMUFile
In reply to Juan QuintelaQEMUFile * 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
@@ -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
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]
PATCH v2 3/3 ds1225y: Use stdio instead of QEMUFile
In reply to Juan QuintelaQEMUFile * 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
@@ -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