[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
Re: [Qemu-arm] [Qemu-devel] [PATCH v2 03/18] cutils: Add qemu_strdup_hexlify() and qemu_strdup_unhexlify()
Tue, 12 Mar 2019 15:04:50 +0100
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
>> 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:
>> 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! :(
- [Qemu-arm] [PATCH v2 00/18] fw_cfg: reduce memleaks, add QMP/HMP info + edk2_add_host_crypto_policy, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-arm] [PATCH v2 04/18] hw/nvram/fw_cfg: Add trace events, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-arm] [PATCH v2 05/18] hw/nvram/fw_cfg: Use the ldst API, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-arm] [PATCH v2 06/18] hw/nvram/fw_cfg: Remove the unnecessary boot_splash_filedata_size, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-arm] [PATCH v2 07/18] hw/nvram/fw_cfg: Add fw_cfg_common_unrealize(), Philippe Mathieu-Daudé, 2019/03/07