From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48356) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbqAa-0006ar-1g for qemu-devel@nongnu.org; Tue, 15 Sep 2015 09:21:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbqAC-0003u1-Fp for qemu-devel@nongnu.org; Tue, 15 Sep 2015 09:20:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41375) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbqAC-0003tE-A2 for qemu-devel@nongnu.org; Tue, 15 Sep 2015 09:20:48 -0400 From: Markus Armbruster References: <1442253477-15422-1-git-send-email-armbru@redhat.com> <1442253477-15422-21-git-send-email-armbru@redhat.com> <20150915113722.GO23145@redhat.com> Date: Tue, 15 Sep 2015 15:20:42 +0200 In-Reply-To: <20150915113722.GO23145@redhat.com> (Daniel P. Berrange's message of "Tue, 15 Sep 2015 12:37:22 +0100") Message-ID: <87wpvr4y51.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v7 20/26] qapi: Make output visitor return qnull() instead of NULL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com "Daniel P. Berrange" 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 >> --- >> 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 Thanks!