Re: [Qemu-devel] [PATCH] qht: do not segfault when gathering stats from an uninitialized qht

[Emilio G. Cota]

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 <address@hidden> 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);