[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] s390: use FILE instead of QEMUFile for creat
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v2] s390: use FILE instead of QEMUFile for creating text file |
Date: |
Mon, 18 Jan 2016 09:50:32 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Fri, Jan 15, 2016 at 04:29:32PM -0700, Eric Blake wrote:
> On 01/12/2016 05:59 AM, Daniel P. Berrange wrote:
> > The s390 skeys monitor command needs to write out a plain text
> > file. Currently it is using the QEMUFile class for this, but
> > work is ongoing to refactor QEMUFile and eliminate much code
> > related to it. The only feature qemu_fopen() gives over fopen()
> > is support for QEMU FD passing, but this can be achieved with
> > qemu_open() + fdopen() too. Switching to regular stdio FILE
> > APIs avoids the need to sprintf via an intermedia buffer which
>
> s/intermedia/intermediate/
>
> > slightly simplifies the code.
> >
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> > hw/s390x/s390-skeys.c | 26 ++++++++++++++------------
> > 1 file changed, 14 insertions(+), 12 deletions(-)
> >
>
> > @@ -124,8 +120,14 @@ void qmp_dump_skeys(const char *filename, Error **errp)
> > return;
> > }
> >
> > - f = qemu_fopen(filename, "wb");
> > + fd = qemu_open(filename, O_WRONLY|O_CREAT|O_TRUNC, 0600);
>
> A strict conversion should probably include O_BINARY for mingw (where
> "wb" turns on binary mode). But maybe we should just make qemu_open()
> itself _always_ provide O_BINARY so that callers don't have to worry
> about it - do we really have a reason to open a file on mingw where we
> want \r\n munged into \n due to text mode?
We're writing text data to the file, so I figured that using binary
mode would be wrong.
>
> > + if (fd < 0) {
> > + error_setg_file_open(errp, errno, filename);
> > + return;
> > + }
> > + f = fdopen(fd, "wb");
> > if (!f) {
> > + close(fd);
> > error_setg_file_open(errp, errno, filename);
>
> close() may corrupt errno, resulting in a report of the wrong message.
> Swap these two lines.
Ok
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|