[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 08/15] qstring: Add qstring_wrap_str()
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v6 08/15] qstring: Add qstring_wrap_str() |
Date: |
Tue, 11 Oct 2016 11:08:45 +0000 |
Hi
On Mon, Oct 10, 2016 at 5:25 PM Eric Blake <address@hidden> wrote:
> 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 <address@hidden>
>
> ---
> 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"));
>
>
You could also remove g_free(tmp_filename) from the normal return path
(this may please static analyzers).
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
>
>
> --
Marc-André Lureau
- [Qemu-devel] [PATCH v6 02/15] qapi: Assert finite use of 'number', (continued)
- [Qemu-devel] [PATCH v6 02/15] qapi: Assert finite use of 'number', Eric Blake, 2016/10/10
- [Qemu-devel] [PATCH v6 12/15] qapi: Support pretty printing in JSON output visitor, Eric Blake, 2016/10/10
- [Qemu-devel] [PATCH v6 11/15] qapi: Add JSON output visitor, Eric Blake, 2016/10/10
- [Qemu-devel] [PATCH v6 10/15] tests: Test qobject_to_json() pretty formatting, Eric Blake, 2016/10/10
- [Qemu-devel] [PATCH v6 14/15] qapi: Add 'any' support to JSON output, Eric Blake, 2016/10/10
- [Qemu-devel] [PATCH v6 13/15] qobject: Implement qobject_to_json() atop JSON visitor, Eric Blake, 2016/10/10
- [Qemu-devel] [PATCH v6 09/15] qobject: Consolidate qobject_to_json() calls, Eric Blake, 2016/10/10
- [Qemu-devel] [PATCH v6 08/15] qstring: Add qstring_wrap_str(), Eric Blake, 2016/10/10
- Re: [Qemu-devel] [PATCH v6 08/15] qstring: Add qstring_wrap_str(),
Marc-André Lureau <=
- [Qemu-devel] [PATCH v6 08.5/15] fixup! qstring: Add qstring_wrap_str(), Eric Blake, 2016/10/11
[Qemu-devel] [PATCH v6 15/15] qemu-img: Use new JSON output formatter, Eric Blake, 2016/10/10
Re: [Qemu-devel] [PATCH v6 00/15] Add qapi-to-JSON visitor, Marc-André Lureau, 2016/10/11