From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34643) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bR62n-00024p-Ia for qemu-devel@nongnu.org; Sat, 23 Jul 2016 19:09:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bR62k-00049z-Ak for qemu-devel@nongnu.org; Sat, 23 Jul 2016 19:09:17 -0400 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:40811) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bR62h-00044v-Tv for qemu-devel@nongnu.org; Sat, 23 Jul 2016 19:09:14 -0400 Date: Sat, 23 Jul 2016 19:09:01 -0400 From: "Emilio G. Cota" Message-ID: <20160723230901.GA10429@flamenco> References: <5791E191.4010404@cn.fujitsu.com> <1469205390-14369-1-git-send-email-cota@braap.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] qht: do not segfault when gathering stats from an uninitialized qht List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Peter Maydell , Changlong Xie , QEMU Developers , Richard Henderson On Sat, Jul 23, 2016 at 12:54:51 +0200, Paolo Bonzini wrote: > On 23/07/2016 12:01, Peter Maydell wrote: > > On 22 July 2016 at 17:36, Emilio G. Cota wrote: > > This looks like we're passing NULL pointers to > > printf %s specifiers. This is undefined behaviour at least > > for POSIX printf, and I can't see anything in the glib > > printf-alike function documentation that gives an extra > > guarantee for this, so it's probably a bad idea. > > > > Printing 'nan' also looks a bit odd, though it's not UB. > > Let's move everything to a new function, so that it's easy to add a > check at the top: I'm OK with this. Regardless, it's probably a good idea to also add the appended to qdist, since as Peter points out passing NULL to printf is undefined. [ Note that we need to return a dup'ed string, since callers are supposed to call g_free() on it when they're done.] If there's interest, I'll send a proper patch on Monday. Thanks, E. diff --git a/tests/test-qdist.c b/tests/test-qdist.c index 0298986..0a03635 100644 --- a/tests/test-qdist.c +++ b/tests/test-qdist.c @@ -360,10 +360,10 @@ static void test_none(void) g_assert(isnan(qdist_xmax(&dist))); pr = qdist_pr_plain(&dist, 0); - g_assert(pr == NULL); + g_assert_cmpstr(pr, ==, "(empty)"); pr = qdist_pr_plain(&dist, 2); - g_assert(pr == NULL); + g_assert_cmpstr(pr, ==, "(empty)"); qdist_destroy(&dist); } diff --git a/util/qdist.c b/util/qdist.c index 56f5738..a43f464 100644 --- a/util/qdist.c +++ b/util/qdist.c @@ -234,7 +234,7 @@ char *qdist_pr_plain(const struct qdist *dist, size_t n) char *ret; if (dist->n == 0) { - return NULL; + return g_strdup("(empty)"); } qdist_bin__internal(&binned, dist, n); ret = qdist_pr_internal(&binned);