Similar to g_string_free(), there are cases where we want to
destroy our reference to a QString while grabbing its contents,
where we want to avoid use-after-free but also avoid a needless
strdup(). But unlike g_string_free(), we are at least sensible
enough to add this feature via a different function name,
instead of trying to overload two completely separate concepts
into a single g_string_free(). Do this by introducing
qstring_consume_str(), then use it where it makes sense.
In the case of monitor.c, note that QString _always_ has a
non-NULL embedded string with at least one byte allocated for
a terminating NUL, so special-casing on length 0 was wasted code.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v4: new patch, suggested by Markus
---
include/qapi/qmp/qstring.h | 1 +
monitor.c | 6 +-----
qobject/qstring.c | 23 +++++++++++++++++++++++
qom/object.c | 3 +--
tests/check-qstring.c | 31 +++++++++++++++++++++++++++----
5 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index a987f3b..2d55c87 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -27,6 +27,7 @@ QString *qstring_from_str(const char *str);
QString *qstring_from_substr(const char *str, int start, int end);
size_t qstring_get_length(const QString *qstring);
const char *qstring_get_str(const QString *qstring);
+char *qstring_consume_str(QString *qstring);
void qstring_append(QString *qstring, const char *str);
void qstring_append_chr(QString *qstring, int c);
void qstring_append_printf(QString *qstring, const char *fmt, ...)
diff --git a/monitor.c b/monitor.c
index 2f56488..934d539 100644
--- a/monitor.c
+++ b/monitor.c
@@ -661,11 +661,7 @@ char *qmp_human_monitor_command(const char *command_line,
bool has_cpu_index,
cur_mon = old_mon;
qemu_mutex_lock(&hmp.out_lock);
- if (qstring_get_length(hmp.outbuf) > 0) {
- output = g_strdup(qstring_get_str(hmp.outbuf));
- } else {
- output = g_strdup("");
- }
+ output = qstring_consume_str(hmp.outbuf);
qemu_mutex_unlock(&hmp.out_lock);
out:
diff --git a/qobject/qstring.c b/qobject/qstring.c
index fbfae27..7a438e9 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -145,6 +145,29 @@ const char *qstring_get_str(const QString *qstring)
}
/**
+ * qstring_consume_str(): Destructively convert a QString to string
+ *
+ * The reference count of @qstring is decremented by one, and the
+ * caller is responsible for calling g_free() on the result. In the
+ * common case where @qstring was not shared, this is faster than
+ * using strdup() on the result of qstring_get_str(); otherwise, the
+ * result is a copy and remaining users of @qstring are unaffected.
+ */
+char *qstring_consume_str(QString *qstring)
+{
+ char *result;
+
+ if (qstring->base.refcnt == 1) {
+ result = qstring->string;
+ qstring->string = NULL;
+ } else {
+ result = g_strdup(qstring->string);
+ }
+ qobject_decref(&qstring->base);
+ return result;
+}
+
+/**
* qstring_destroy_obj(): Free all memory allocated by a QString
* object
*/
diff --git a/qom/object.c b/qom/object.c
index 26e7d58..7960729 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1106,10 +1106,9 @@ char *object_property_get_str(Object *obj, const char
*name,
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "string");
retval = NULL;
} else {
- retval = g_strdup(qstring_get_str(qstring));
+ retval = qstring_consume_str(qstring);
}
- QDECREF(qstring);
return retval;
}
diff --git a/tests/check-qstring.c b/tests/check-qstring.c
index 9877b42..e6f58e0 100644
--- a/tests/check-qstring.c
+++ b/tests/check-qstring.c
@@ -1,7 +1,7 @@
/*
* QString unit-tests.
*
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009-2016 Red Hat Inc.
*
* Authors:
* Luiz Capitulino <lcapitul...@redhat.com>
@@ -32,9 +32,7 @@ static void qstring_from_str_test(void)
g_assert(strcmp(str, qstring->string) == 0);
g_assert(qobject_type(QOBJECT(qstring)) == QTYPE_QSTRING);
- // destroy doesn't exit yet
- g_free(qstring->string);
- g_free(qstring);
+ QDECREF(qstring);
}
static void qstring_destroy_test(void)
@@ -56,6 +54,30 @@ static void qstring_get_str_test(void)
QDECREF(qstring);
}
+static void qstring_consume_str_test(void)
+{
+ QString *qstring;
+ char *ret_str;
+ const char *str = "QEMU/KVM";
+ char *ptr;
+
+ qstring = qstring_from_str(str);
+ QINCREF(qstring);
+ g_assert_cmpint(qstring->base.refcnt, ==, 2);
+ ptr = qstring->string;
+
+ ret_str = qstring_consume_str(qstring);
+ g_assert_cmpint(qstring->base.refcnt, ==, 1);
+ g_assert(ret_str != ptr);
+ g_assert_cmpstr(ret_str, ==, str);
+ g_free(ret_str);
+
+ ret_str = qstring_consume_str(qstring);
+ g_assert(ret_str == ptr);
+ g_assert_cmpstr(ret_str, ==, str);
+ g_free(ret_str);
+}
+
static void qstring_append_chr_test(void)
{
int i;
@@ -103,6 +125,7 @@ int main(int argc, char **argv)
g_test_add_func("/public/append_chr", qstring_append_chr_test);
g_test_add_func("/public/from_substr", qstring_from_substr_test);
g_test_add_func("/public/to_qstring", qobject_to_qstring_test);
+ g_test_add_func("/public/consume_str", qstring_consume_str_test);
return g_test_run();
}
--
2.5.5
