Re: [Qemu-devel] [PATCH v2] s390: use FILE instead of QEMUFile for creating text file

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

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 <berrange@redhat.com>
> ---
>  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?

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]