Wenchao Xia <xiaw...@linux.vnet.ibm.com> writes: >>> >>> -void bdrv_image_info_dump(ImageInfo *info) >>> +void bdrv_image_info_dump(GString *buf, ImageInfo *info) >>> { >>> char size_buf[128], dsize_buf[128]; >>> if (!info->has_actual_size) { >>> @@ -370,43 +369,48 @@ void bdrv_image_info_dump(ImageInfo *info) >> >> I don't like this change, because it introduces buffering for no >> discernible reason. Unless you can show me one, I'd like you to keep >> printing directly. >> > HMP code later need to call this function, and then print buf to > monitor console, which is the goal of this patch.
Aha, the actual problem is "print either to stdout or to current monitor", so that we can share code among qemu-img and monitor commands. Such sharing is good, and the problem is real. bdrv_snapshot_dump() solves it this way: print to a fixed-size buffer, which the caller either prints to stdout (qemu-img) or to the current monitor (info snapshots). Your patch makes the buffer flexible. Improvement. But in my opinion, the detour through the buffer is silly. Why not simply use a print function that does the right thing? We already have such a function for "print either to stderr or to current monitor": error_printf(). Could easily add one for stdout. [...]