From f9dbc19e8bf58d0cbc830083352475bb16f315c4 Mon Sep 17 00:00:00 2001 From: "Emilio G. Cota" Date: Mon, 25 Jul 2016 11:03:43 -0400 Subject: qdist: fix memory leak during binning In qdist_bin__internal(), to->entries is initialized to a 1-element array, which we then leak when n == from->n. Fix it. Signed-off-by: Emilio G. Cota Message-Id: <1469459025-23606-2-git-send-email-cota@braap.org> Signed-off-by: Paolo Bonzini --- util/qdist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'util/qdist.c') diff --git a/util/qdist.c b/util/qdist.c index 56f573837d..eb2236cdc8 100644 --- a/util/qdist.c +++ b/util/qdist.c @@ -188,7 +188,7 @@ void qdist_bin__internal(struct qdist *to, const struct qdist *from, size_t n) } } /* they're equally spaced, so copy the dist and bail out */ - to->entries = g_new(struct qdist_entry, from->n); + to->entries = g_realloc_n(to->entries, n, sizeof(*to->entries)); to->n = from->n; memcpy(to->entries, from->entries, sizeof(*to->entries) * to->n); return; -- cgit 1.4.1 From 071d4054770205ddb8a58a9e2735069d8fe52af1 Mon Sep 17 00:00:00 2001 From: "Emilio G. Cota" Date: Mon, 25 Jul 2016 11:03:44 -0400 Subject: qdist: use g_renew and g_new instead of g_realloc and g_malloc. This is safer against overflow. g_renew is available in all version of glib, while g_realloc_n is only available in 2.24. Signed-off-by: Emilio G. Cota Message-Id: <1469459025-23606-3-git-send-email-cota@braap.org> [Rewritten to use g_new/g_renew. - Paolo] Signed-off-by: Paolo Bonzini --- util/qdist.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'util/qdist.c') diff --git a/util/qdist.c b/util/qdist.c index eb2236cdc8..e95722b8a1 100644 --- a/util/qdist.c +++ b/util/qdist.c @@ -16,7 +16,7 @@ void qdist_init(struct qdist *dist) { - dist->entries = g_malloc(sizeof(*dist->entries)); + dist->entries = g_new(struct qdist_entry, 1); dist->size = 1; dist->n = 0; } @@ -62,8 +62,7 @@ void qdist_add(struct qdist *dist, double x, long count) if (unlikely(dist->n == dist->size)) { dist->size *= 2; - dist->entries = g_realloc(dist->entries, - sizeof(*dist->entries) * (dist->size)); + dist->entries = g_renew(struct qdist_entry, dist->entries, dist->size); } dist->n++; entry = &dist->entries[dist->n - 1]; @@ -188,7 +187,7 @@ void qdist_bin__internal(struct qdist *to, const struct qdist *from, size_t n) } } /* they're equally spaced, so copy the dist and bail out */ - to->entries = g_realloc_n(to->entries, n, sizeof(*to->entries)); + to->entries = g_renew(struct qdist_entry, to->entries, n); to->n = from->n; memcpy(to->entries, from->entries, sizeof(*to->entries) * to->n); return; -- cgit 1.4.1 From 11b7b07f8a15879134a54e73fade98d5e11e04f8 Mon Sep 17 00:00:00 2001 From: "Emilio G. Cota" Date: Mon, 25 Jul 2016 11:03:45 -0400 Subject: qdist: return "(empty)" instead of NULL when printing an empty dist Printf'ing a NULL string is undefined behaviour. Avoid it. Reported-by: Peter Maydell Signed-off-by: Emilio G. Cota Message-Id: <1469459025-23606-4-git-send-email-cota@braap.org> Signed-off-by: Paolo Bonzini --- tests/test-qdist.c | 10 ++++++++-- util/qdist.c | 6 ++++-- 2 files changed, 12 insertions(+), 4 deletions(-) (limited to 'util/qdist.c') diff --git a/tests/test-qdist.c b/tests/test-qdist.c index 0298986ac9..9541ce31eb 100644 --- a/tests/test-qdist.c +++ b/tests/test-qdist.c @@ -360,10 +360,16 @@ 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)"); + g_free(pr); pr = qdist_pr_plain(&dist, 2); - g_assert(pr == NULL); + g_assert_cmpstr(pr, ==, "(empty)"); + g_free(pr); + + pr = qdist_pr(&dist, 0, QDIST_PR_BORDER); + g_assert_cmpstr(pr, ==, "(empty)"); + g_free(pr); qdist_destroy(&dist); } diff --git a/util/qdist.c b/util/qdist.c index e95722b8a1..5f75e24c29 100644 --- a/util/qdist.c +++ b/util/qdist.c @@ -14,6 +14,8 @@ #define NAN (0.0 / 0.0) #endif +#define QDIST_EMPTY_STR "(empty)" + void qdist_init(struct qdist *dist) { dist->entries = g_new(struct qdist_entry, 1); @@ -233,7 +235,7 @@ char *qdist_pr_plain(const struct qdist *dist, size_t n) char *ret; if (dist->n == 0) { - return NULL; + return g_strdup(QDIST_EMPTY_STR); } qdist_bin__internal(&binned, dist, n); ret = qdist_pr_internal(&binned); @@ -308,7 +310,7 @@ char *qdist_pr(const struct qdist *dist, size_t n_bins, uint32_t opt) GString *s; if (dist->n == 0) { - return NULL; + return g_strdup(QDIST_EMPTY_STR); } s = g_string_new(""); -- cgit 1.4.1