Peter Xu <pet...@redhat.com> writes:
> Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
> CC: Markus Armbruster <arm...@redhat.com>
> CC: Kevin Wolf <kw...@redhat.com>
> CC: qemu-bl...@nongnu.org
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> block/qapi.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index db2d3fb..687e577 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -638,9 +638,12 @@ static void dump_qdict(fprintf_function func_fprintf,
> void *f, int indentation,
> QType type = qobject_type(entry->value);
> bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> const char *format = composite ? "%*s%s:\n" : "%*s%s: ";
Unrelated to your patch: ugh!
Printf formats should be literals whenever possible, to make it easy for
the compiler to warn you when you screw up. It's trivially possible
here! Instead of
func_fprintf(f, format, indentation * 4, "", key);
do
func_fprintf(f, "%*s%s:%c", indentation * 4, "", key,
composite ? '\n', ' ');;
> - char key[strlen(entry->key) + 1];
> +#define __KEY_LEN (256)
> + char key[__KEY_LEN];
> int i;
>
> + assert(strlen(entry->key) + 1 <= __KEY_LEN);
> +#undef __KEY_LEN
> /* replace dashes with spaces in key (variable) names */
> for (i = 0; entry->key[i]; i++) {
> key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
I'm afraid this isn't a good idea. It relies on the non-local argument
that nobody will ever put a key longer than 255 into a qdict that gets
dumped. That may even be the case, but you need to *prove* it, not just
assert it. The weakest acceptable proof might be assertions in every
place that put keys into a dict that might get dumped. I suspect that's
practical and maintainable only if there's a single place that does it.
If this was a good idea, I'd recommend to avoid the awkward macro:
char key[256];
int i;
assert(strlen(entry->key) + 1 <= ARRAY_SIZE(key));
There are several other ways to limit the stack usage:
1. Move the array from stack to heap. Fine unless it's on a hot path.
As far as I can tell, this dumping business is for HMP and qemu-io,
i.e. not hot.
2. Use a stack array for short strings, switch to the heap for large
strings. More complex, but may be worth it on a hot path where most
strings are short.
3. Instead of creating a new string with '-' replaced for printing,
print characters. Can be okay with buffered I/O, but obviously not
on a hot path.
4. Like 3., but print substrings not containing '-'.
