[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [Qemu-devel] [PATCH v2 03/18] cutils: Add qemu_strdup_hex

From: Laszlo Ersek
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH v2 03/18] cutils: Add qemu_strdup_hexlify() and qemu_strdup_unhexlify()
Date: Tue, 12 Mar 2019 15:04:50 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

On 03/09/19 15:32, Markus Armbruster wrote:
> Laszlo Ersek <address@hidden> writes:

>> (5) Most importantly: I don't think we need this patch.
>> First, AFAICS, the unhex function is never used in the series, and no
>> unit test is being added for it. That makes it a bad candidate for
>> "include/qemu/cutils.h".
>> Second, while the hex function is used in PATCH v2 13/18
>> ("hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command"), the documentation in
>> that patch and the logic in the patch are inconsistent. The
>> documentation -- i.e. both the commit message and the "misc.json" change
>> -- say that "FirmwareConfigurationItem.data" is unused (not populated).
>> However, that's exactly what create_qmp_fw_cfg_item() uses the hex
>> function for.
>> Third, if we do decide that the QMP command should output the fw_cfg
>> binary data, then the QMP tradition (to my knowledge) has been to use
>> base64 encoding. GLib provides helpers for base64:
>> https://developer.gnome.org/glib/stable/glib-Base64-Encoding.html
>> and you can see examples of it being used in e.g.
>> (a) qmp_ringbuf_read() [chardev/char-ringbuf.c] -- the  @ringbuf-read
>> command is defined in "qapi/char.json"
>> (b) qmp_guest_exec_status() [qga/commands.c] -- the @guest-exec-status
>> command is defined in "qga/qapi-schema.json".
> Yes.  I wish you had wrote that first, saving me the trouble of looking
> at the patch.

You are right, I'm sorry! :(

reply via email to

[Prev in Thread] Current Thread [Next in Thread]