Several spots in the code malloc a string, then wrap it in a QString, which has to duplicate the input. Adding a new constructor with transfer semantics makes this pattern more efficient, comparable to the just-added transfer semantics to go from QString back to raw string. Use the new qstring_wrap_str() where it makes sense.
The new test still passes under valgrind. Signed-off-by: Eric Blake <ebl...@redhat.com> --- v6: don't auto-convert NULL into "" [no v5 due to series split] v4: new patch --- include/qapi/qmp/qstring.h | 1 + block.c | 3 ++- block/archipelago.c | 6 ++---- blockdev.c | 3 +-- qobject/qstring.c | 20 ++++++++++++++++++++ tests/check-qstring.c | 15 +++++++++++++++ 6 files changed, 41 insertions(+), 7 deletions(-) diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h index 2d55c87..97cf776 100644 --- a/include/qapi/qmp/qstring.h +++ b/include/qapi/qmp/qstring.h @@ -25,6 +25,7 @@ typedef struct QString { QString *qstring_new(void); QString *qstring_from_str(const char *str); QString *qstring_from_substr(const char *str, int start, int end); +QString *qstring_wrap_str(char *str); size_t qstring_get_length(const QString *qstring); const char *qstring_get_str(const QString *qstring); char *qstring_consume_str(QString *qstring); diff --git a/block.c b/block.c index bb1f1ec..8a2876f 100644 --- a/block.c +++ b/block.c @@ -1640,7 +1640,8 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs, qdict_put(snapshot_options, "file.driver", qstring_from_str("file")); qdict_put(snapshot_options, "file.filename", - qstring_from_str(tmp_filename)); + qstring_wrap_str(tmp_filename)); + tmp_filename = NULL; qdict_put(snapshot_options, "driver", qstring_from_str("qcow2")); diff --git a/block/archipelago.c b/block/archipelago.c index 37b8aca..ac047e4 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -426,13 +426,11 @@ static void archipelago_parse_filename(const char *filename, QDict *options, parse_filename_opts(filename, errp, &volume, &segment_name, &mport, &vport); if (volume) { - qdict_put(options, ARCHIPELAGO_OPT_VOLUME, qstring_from_str(volume)); - g_free(volume); + qdict_put(options, ARCHIPELAGO_OPT_VOLUME, qstring_wrap_str(volume)); } if (segment_name) { qdict_put(options, ARCHIPELAGO_OPT_SEGMENT, - qstring_from_str(segment_name)); - g_free(segment_name); + qstring_wrap_str(segment_name)); } if (mport != NoPort) { qdict_put(options, ARCHIPELAGO_OPT_MPORT, qint_from_int(mport)); diff --git a/blockdev.c b/blockdev.c index 07ec733..35cd905 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1026,8 +1026,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) new_id = g_strdup_printf("%s%s%i", if_name[type], mediastr, unit_id); } - qdict_put(bs_opts, "id", qstring_from_str(new_id)); - g_free(new_id); + qdict_put(bs_opts, "id", qstring_wrap_str(new_id)); } /* Add virtio block device */ diff --git a/qobject/qstring.c b/qobject/qstring.c index 7a438e9..a64373a 100644 --- a/qobject/qstring.c +++ b/qobject/qstring.c @@ -66,6 +66,26 @@ QString *qstring_from_str(const char *str) return qstring_from_substr(str, 0, strlen(str) - 1); } +/** + * qstring_wrap_str(): Create a new QString around a malloc'd C string + * + * Returns a strong reference, and caller must not use @str any more. + * @str must not be NULL. + */ +QString *qstring_wrap_str(char *str) +{ + QString *qstring; + + qstring = g_malloc(sizeof(*qstring)); + qobject_init(QOBJECT(qstring), QTYPE_QSTRING); + + assert(str); + qstring->string = str; + qstring->capacity = qstring->length = strlen(str); + + return qstring; +} + static void capacity_increase(QString *qstring, size_t len) { if (qstring->capacity < (qstring->length + len)) { diff --git a/tests/check-qstring.c b/tests/check-qstring.c index 11823c2..0c427c7 100644 --- a/tests/check-qstring.c +++ b/tests/check-qstring.c @@ -34,6 +34,20 @@ static void qstring_from_str_test(void) QDECREF(qstring); } +static void qstring_wrap_str_test(void) +{ + QString *qstring; + char *str = g_strdup("QEMU"); + + qstring = qstring_wrap_str(str); + g_assert(qstring != NULL); + g_assert(qstring->base.refcnt == 1); + g_assert(strcmp("QEMU", qstring->string) == 0); + g_assert(qobject_type(QOBJECT(qstring)) == QTYPE_QSTRING); + + QDECREF(qstring); +} + static void qstring_destroy_test(void) { QString *qstring = qstring_from_str("destroy test"); @@ -119,6 +133,7 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); g_test_add_func("/public/from_str", qstring_from_str_test); + g_test_add_func("/public/wrap_str", qstring_wrap_str_test); g_test_add_func("/public/destroy", qstring_destroy_test); g_test_add_func("/public/get_str", qstring_get_str_test); g_test_add_func("/public/append_chr", qstring_append_chr_test); -- 2.7.4