Fam Zheng <f...@redhat.com> writes: > On Tue, 03/08 09:12, Markus Armbruster wrote: >> 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: > > Also I think the double underscore identifiers are considered reserved in C, > no?
Correct. C99 7.1.3 Reserved identifiers: All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use. [...]