From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aL6SO-0003o9-1b for qemu-devel@nongnu.org; Mon, 18 Jan 2016 04:50:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aL6SK-0003FP-Rz for qemu-devel@nongnu.org; Mon, 18 Jan 2016 04:50:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55456) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aL6SK-0003FE-MW for qemu-devel@nongnu.org; Mon, 18 Jan 2016 04:50:36 -0500 Date: Mon, 18 Jan 2016 09:50:32 +0000 From: "Daniel P. Berrange" Message-ID: <20160118095032.GA13395@redhat.com> References: <1452603559-13685-1-git-send-email-berrange@redhat.com> <569980DC.5030707@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <569980DC.5030707@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2] s390: use FILE instead of QEMUFile for creating text file Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Cornelia Huck , qemu-devel@nongnu.org, Richard Henderson 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 > > --- > > 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 :|