[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 03/18] cutils: Add qemu_strdup_hexlify() and
From: |
Laszlo Ersek |
Subject: |
Re: [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! :(
Laszlo
- [Qemu-devel] [PATCH v2 00/18] fw_cfg: reduce memleaks, add QMP/HMP info + edk2_add_host_crypto_policy, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-devel] [PATCH v2 04/18] hw/nvram/fw_cfg: Add trace events, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-devel] [PATCH v2 05/18] hw/nvram/fw_cfg: Use the ldst API, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-devel] [PATCH v2 06/18] hw/nvram/fw_cfg: Remove the unnecessary boot_splash_filedata_size, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-devel] [PATCH v2 07/18] hw/nvram/fw_cfg: Add fw_cfg_common_unrealize(), Philippe Mathieu-Daudé, 2019/03/07