Re: [Qemu-devel] [PATCH v1 01/22] s390: use FILE instead of QEMUFile for creating text file

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Daniel P. Berrange (berrange@redhat.com) wrote:
> The s390 skeys monitor command needs to write out a plain text
> file. Currently it is using the QEMUFile class for this. There
> is no real benefit to this, and the downside is that it needs to
> snprintf via an intermediate buffer. Switching to regular FILE
> objects simplifies the code.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave

> ---
>  hw/s390x/s390-skeys.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index 539ef6d..637ccf5 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -45,15 +45,11 @@ void s390_skeys_init(void)
>      qdev_init_nofail(DEVICE(obj));
>  }
>  
> -static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
> +static void write_keys(FILE *f, uint8_t *keys, uint64_t startgfn,
>                         uint64_t count, Error **errp)
>  {
>      uint64_t curpage = startgfn;
>      uint64_t maxpage = curpage + count - 1;
> -    const char *fmt = "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
> -                      " ch=%d, reserved=%d\n";
> -    char buf[128];
> -    int len;
>  
>      for (; curpage <= maxpage; curpage++) {
>          uint8_t acc = (*keys & 0xF0) >> 4;
> @@ -62,10 +58,9 @@ static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
>          int ch = (*keys & 0x02);
>          int res = (*keys & 0x01);
>  
> -        len = snprintf(buf, sizeof(buf), fmt, curpage,
> -                       *keys, acc, fp, ref, ch, res);
> -        assert(len < sizeof(buf));
> -        qemu_put_buffer(f, (uint8_t *)buf, len);
> +        fprintf(f, "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
> +                " ch=%d, reserved=%d\n",
> +                curpage, *keys, acc, fp, ref, ch, res);
>          keys++;
>      }
>  }
> @@ -115,7 +110,7 @@ void qmp_dump_skeys(const char *filename, Error **errp)
>      vaddr cur_gfn = 0;
>      uint8_t *buf;
>      int ret;
> -    QEMUFile *f;
> +    FILE *f;
>  
>      /* Quick check to see if guest is using storage keys*/
>      if (!skeyclass->skeys_enabled(ss)) {
> @@ -124,7 +119,7 @@ void qmp_dump_skeys(const char *filename, Error **errp)
>          return;
>      }
>  
> -    f = qemu_fopen(filename, "wb");
> +    f = fopen(filename, "wb");
>      if (!f) {
>          error_setg_file_open(errp, errno, filename);
>          return;
> @@ -161,7 +156,7 @@ out_free:
>      error_propagate(errp, lerr);
>      g_free(buf);
>  out:
> -    qemu_fclose(f);
> +    fclose(f);
>  }
>  
>  static void qemu_s390_skeys_init(Object *obj)
> -- 
> 2.5.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK