Re: [Qemu-devel] [PATCH for-1.4 02/12] qmp: Clean up design of memchar-read

[Eric Blake]

On 02/05/2013 09:22 AM, Markus Armbruster wrote:
> The data returned has a well-defined size, which makes the size
> returned along with it redundant at best.  Drop it.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  hmp.c            | 11 ++++-------
>  qapi-schema.json | 18 ++----------------
>  qemu-char.c      | 21 +++++++++------------
>  qmp-commands.hx  |  2 +-
>  4 files changed, 16 insertions(+), 36 deletions(-)

What happens if the data to be read includes a NUL byte, but the user
didn't request base64 encoding?

> -    meminfo->count = cirmem_chr_read(chr, read_data, size);
> +    cirmem_chr_read(chr, read_data, size);
>  
>      if (has_format && (format == DATA_FORMAT_BASE64)) {
> -        meminfo->data = g_base64_encode(read_data, (size_t)meminfo->count);
> +        data = g_base64_encode(read_data, count);
>      } else {
> -        meminfo->data = (char *)read_data;
> +        data = (char *)read_data;
>      }

This makes it look like the user just gets a truncated read, but is the
remainder of the buffer lost, or will the next attempt get it as well?
Would it be better to instead return an error that the user's requested
encoding is insufficient for the data that is trying to be read?

At any rate, I agree with the idea of this patch in simplifying the API,
and concur that we must solve it before 1.4 locks us in to a bad design.

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

signature.asc
Description: OpenPGP digital signature