"Daniel P. Berrange" <berra...@redhat.com> writes: > On Mon, Sep 14, 2015 at 07:57:51PM +0200, Markus Armbruster wrote: >> Before commit 1d10b44, it crashed. Since then, it returns NULL, with >> a FIXME comment. The FIXME is valid: code that assumes QObject * >> can't be null exists. I'm not aware of a way to feed this problematic >> return value to code that actually chokes on null in the current code, >> but the next few commits will create one, failing "make check". >> >> Commit 481b002 solved a very similar problem by introducing a special >> null QObject. Using this special null QObject is clearly the right >> way to resolve this FIXME, so do that, and update the test >> accordingly. >> >> However, the patch isn't quite right: it messes up the reference >> counting. After about SIZE_MAX visits, the reference counter >> overflows, failing the assertion in qnull_destroy_obj(). Because >> that's many orders of magnitude more visits of nulls than we expect, >> we take this patch despite its flaws, to get the QMP introspection >> stuff in without further delay. >> >> Naturally, we'll have to fix it for real before the release. > > Do we actually ever get near to SIZE_MAX visits ? If not, then > it would not seem critical to fix before release, as this is > just the generator code
SIZE_MAX visits seem unlikely even when SIZE_MAX is only 2^32-1. It would be fatal, though: QEMU would crash. I'll reword to "we'll want to fix it". >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> qapi/qmp-output-visitor.c | 8 ++++++-- >> tests/test-qmp-output-visitor.c | 3 ++- >> 2 files changed, 8 insertions(+), 3 deletions(-) > > Reviewed-by: Daniel P. Berrange <berra...@redhat.com> Thanks!