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 <c...@braap.org> 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);