From 42b64bfc285d5d662d96b0364847431862687228 Mon Sep 17 00:00:00 2001 From: George Kiagiadakis Date: Tue, 14 Nov 2023 12:36:10 +0200 Subject: [PATCH] spa-json: rename _from_string() to _wrap_string() and add new "from" variants The previous naming convention was confusing because it did not make it explicit that the string is not being copied. We had this wrong already in the Lua bindings and thanks to some miracle it hasn't backfired so far (it was using the "wrap" behaviour with a string that doesn't stay alive). In some places we actually need the "copy" behaviour and in some other places we need the "wrap" behaviour, so let's have both variants available. --- lib/wp/conf.c | 2 +- lib/wp/json-utils.c | 2 +- lib/wp/settings.c | 4 +-- lib/wp/spa-json.c | 47 +++++++++++++++++++++++++----- lib/wp/spa-json.h | 6 ++++ modules/module-default-nodes-api.c | 4 +-- modules/module-settings.c | 2 +- src/tools/wpexec.c | 2 +- tests/wp/conf.c | 2 +- tests/wp/json-utils.c | 4 +-- tests/wp/spa-json.c | 10 +++---- 11 files changed, 61 insertions(+), 24 deletions(-) diff --git a/lib/wp/conf.c b/lib/wp/conf.c index 994700e4..f1db5151 100644 --- a/lib/wp/conf.c +++ b/lib/wp/conf.c @@ -157,7 +157,7 @@ merge_section_cb (void *data, const char *location, const char *section, location); /* Only allow sections to be objects or arrays */ - json = wp_spa_json_new_from_stringn (str, len); + json = wp_spa_json_new_wrap_stringn (str, len); if (!wp_spa_json_is_container (json)) { wp_warning ( "skipping section %s from %s as it is not JSON object or array", diff --git a/lib/wp/json-utils.c b/lib/wp/json-utils.c index a0515b7c..39f7f628 100644 --- a/lib/wp/json-utils.c +++ b/lib/wp/json-utils.c @@ -29,7 +29,7 @@ match_rules_cb (void *data, const char *location, const char *action, const char *str, size_t len) { struct match_rules_cb_data *cb_data = data; - g_autoptr (WpSpaJson) json = wp_spa_json_new_from_stringn (str, len); + g_autoptr (WpSpaJson) json = wp_spa_json_new_wrap_stringn (str, len); return cb_data->callback (cb_data->data, action, json, cb_data->error) ? 0 : -EPIPE; } diff --git a/lib/wp/settings.c b/lib/wp/settings.c index af35be1c..a4f232ff 100644 --- a/lib/wp/settings.c +++ b/lib/wp/settings.c @@ -163,7 +163,7 @@ wp_settings_get (WpSettings *self, const gchar *setting) return NULL; value = wp_properties_get (self->settings, setting); - return value ? wp_spa_json_new_from_string (value) : NULL; + return value ? wp_spa_json_new_wrap_string (value) : NULL; } enum { @@ -271,7 +271,7 @@ on_metadata_changed (WpMetadata *m, guint32 subject, g_value_set_object (&values[0], self); g_value_set_string (&values[1], setting); - json = new_value ? wp_spa_json_new_from_string (new_value) : NULL; + json = new_value ? wp_spa_json_new_wrap_string (new_value) : NULL; g_value_set_boxed (&values[2], json); g_closure_invoke (cb->closure, NULL, 3, values, NULL); diff --git a/lib/wp/spa-json.c b/lib/wp/spa-json.c index 0d971619..0d4d6a6b 100644 --- a/lib/wp/spa-json.c +++ b/lib/wp/spa-json.c @@ -184,13 +184,14 @@ wp_spa_json_new (const gchar *data, size_t size) * * \ingroup wpspajson * \param json_str a JSON string - * \returns a new WpSpaJson that references the data in \a json_str. \a json_str - * is not copied, so it needs to stay alive. + * \returns a new WpSpaJson; unlike the "wrap" variants, this function copies + * the data in \a json_str, so it does not need to stay alive. + * \since 0.5.0 */ -WpSpaJson * -wp_spa_json_new_from_string (const gchar *json_str) +WP_API +WpSpaJson * wp_spa_json_new_from_string (const gchar *json_str) { - return wp_spa_json_new_from_stringn(json_str, strlen (json_str)); + return wp_spa_json_new (json_str, strlen (json_str)); } /*! @@ -199,13 +200,43 @@ wp_spa_json_new_from_string (const gchar *json_str) * \ingroup wpspajson * \param json_str a JSON string * \param len the specific length of the string + * \returns a new WpSpaJson; unlike the "wrap" variants, this function copies + * the data in \a json_str, so it does not need to stay alive. + * \since 0.5.0 + */ +WP_API +WpSpaJson * wp_spa_json_new_from_stringn (const gchar *json_str, size_t len) +{ + return wp_spa_json_new (json_str, len); +} + +/*! + * \brief Constructs a new WpSpaJson that wraps a JSON string. + * + * \ingroup wpspajson + * \param json_str a JSON string * \returns a new WpSpaJson that references the data in \a json_str. \a json_str * is not copied, so it needs to stay alive. - * - * \since 0.4.10 + * \since 0.5.0 */ WpSpaJson * -wp_spa_json_new_from_stringn (const gchar *json_str, size_t len) +wp_spa_json_new_wrap_string (const gchar *json_str) +{ + return wp_spa_json_new_wrap_stringn(json_str, strlen (json_str)); +} + +/*! + * \brief Constructs a new WpSpaJson that wraps a JSON string with specific length. + * + * \ingroup wpspajson + * \param json_str a JSON string + * \param len the specific length of the string + * \returns a new WpSpaJson that references the data in \a json_str. \a json_str + * is not copied, so it needs to stay alive. + * \since 0.5.0 + */ +WpSpaJson * +wp_spa_json_new_wrap_stringn (const gchar *json_str, size_t len) { WpSpaJson *self = g_slice_new0 (WpSpaJson); g_ref_count_init (&self->ref); diff --git a/lib/wp/spa-json.h b/lib/wp/spa-json.h index d182c3cf..927ead1f 100644 --- a/lib/wp/spa-json.h +++ b/lib/wp/spa-json.h @@ -40,6 +40,12 @@ WpSpaJson * wp_spa_json_new_from_string (const gchar *json_str); WP_API WpSpaJson * wp_spa_json_new_from_stringn (const gchar *json_str, size_t len); +WP_API +WpSpaJson * wp_spa_json_new_wrap_string (const gchar *json_str); + +WP_API +WpSpaJson * wp_spa_json_new_wrap_stringn (const gchar *json_str, size_t len); + WP_API WpSpaJson * wp_spa_json_new_wrap (struct spa_json *json); diff --git a/modules/module-default-nodes-api.c b/modules/module-default-nodes-api.c index f7e7fc4b..642db9fb 100644 --- a/modules/module-default-nodes-api.c +++ b/modules/module-default-nodes-api.c @@ -114,7 +114,7 @@ on_metadata_changed (WpMetadata *m, guint32 subject, if (!g_strcmp0 (key, DEFAULT_KEY[i])) { if (value && !g_strcmp0 (type, "Spa:String:JSON")) { - g_autoptr (WpSpaJson) json = wp_spa_json_new_from_string (value); + g_autoptr (WpSpaJson) json = wp_spa_json_new_wrap_string (value); wp_spa_json_object_get (json, "name", "s", &new_value, NULL); } @@ -129,7 +129,7 @@ on_metadata_changed (WpMetadata *m, guint32 subject, } else if (!g_strcmp0 (key, DEFAULT_CONFIG_KEY[i])) { if (value && !g_strcmp0 (type, "Spa:String:JSON")) { - g_autoptr (WpSpaJson) json = wp_spa_json_new_from_string (value); + g_autoptr (WpSpaJson) json = wp_spa_json_new_wrap_string (value); wp_spa_json_object_get (json, "name", "s", &new_value, NULL); } diff --git a/modules/module-settings.c b/modules/module-settings.c index 10dbeebf..35858e06 100644 --- a/modules/module-settings.c +++ b/modules/module-settings.c @@ -163,7 +163,7 @@ is_persistent_settings_enabled (WpProperties *settings) { if (!val_str) return FALSE; - val = wp_spa_json_new_from_string (val_str); + val = wp_spa_json_new_wrap_string (val_str); if (val && !wp_spa_json_parse_boolean (val, &res)) wp_warning ("Could not parse " PERSISTENT_SETTING " in main configuration"); diff --git a/src/tools/wpexec.c b/src/tools/wpexec.c index 4a7950e0..fdf4d1cf 100644 --- a/src/tools/wpexec.c +++ b/src/tools/wpexec.c @@ -43,7 +43,7 @@ parse_exec_script_arg (const gchar *option_name, const gchar *value, /* the second argument is a json object with script arguments */ if (!exec_args) { exec_args_s = g_strdup (value); - exec_args = wp_spa_json_new_from_string (exec_args_s); + exec_args = wp_spa_json_new_wrap_string (exec_args_s); if (!exec_args || !wp_spa_json_is_object (exec_args)) { g_set_error (error, WP_DOMAIN_DAEMON, WP_EXIT_USAGE, "script argument must be a JSON object"); diff --git a/tests/wp/conf.c b/tests/wp/conf.c index 989083a5..5e0ce44f 100644 --- a/tests/wp/conf.c +++ b/tests/wp/conf.c @@ -167,7 +167,7 @@ test_conf_basic (TestConfFixture *f, gconstpointer data) /* Fallback */ { - g_autoptr (WpSpaJson) fallback = wp_spa_json_new_from_string ("{key1 = 3"); + g_autoptr (WpSpaJson) fallback = wp_spa_json_new_wrap_string ("{key1 = 3"); g_assert_nonnull (fallback); g_autoptr (WpSpaJson) s = wp_conf_get_section (f->conf, diff --git a/tests/wp/json-utils.c b/tests/wp/json-utils.c index 6c56a32a..c96bb601 100644 --- a/tests/wp/json-utils.c +++ b/tests/wp/json-utils.c @@ -42,7 +42,7 @@ test_match_rules_update_properties (void) " }" "]"; - g_autoptr (WpSpaJson) rules = wp_spa_json_new_from_stringn (rules_json_string, + g_autoptr (WpSpaJson) rules = wp_spa_json_new_wrap_stringn (rules_json_string, strlen (rules_json_string)); g_assert_nonnull (rules); @@ -251,7 +251,7 @@ test_match_rules (void) " }" "]"; - g_autoptr (WpSpaJson) rules = wp_spa_json_new_from_stringn (rules_json_string, + g_autoptr (WpSpaJson) rules = wp_spa_json_new_wrap_stringn (rules_json_string, strlen (rules_json_string)); g_assert_nonnull (rules); diff --git a/tests/wp/spa-json.c b/tests/wp/spa-json.c index 1a89e39b..74423f38 100644 --- a/tests/wp/spa-json.c +++ b/tests/wp/spa-json.c @@ -817,7 +817,7 @@ static void test_spa_json_nested2 (void) { const gchar json_str[] = "[[[[1], [2]], [3]], [4]]"; - g_autoptr (WpSpaJson) json = wp_spa_json_new_from_string (json_str); + g_autoptr (WpSpaJson) json = wp_spa_json_new_wrap_string (json_str); g_assert_true (wp_spa_json_is_array (json)); g_assert_cmpmem (wp_spa_json_get_data (json), wp_spa_json_get_size (json), @@ -987,7 +987,7 @@ test_spa_json_nested3 (void) { const gchar json_str[] = "{ test-setting-json3: { key1: \"value\", key2: 2, key3: true } }"; - g_autoptr (WpSpaJson) json = wp_spa_json_new_from_string (json_str); + g_autoptr (WpSpaJson) json = wp_spa_json_new_wrap_string (json_str); g_assert_nonnull (json); g_assert_true (wp_spa_json_is_object (json)); @@ -1023,7 +1023,7 @@ test_spa_json_ownership (void) { const gchar json_str[] = "{\"name\":\"John\", \"age\":30, \"car\":null}"; - json = wp_spa_json_new_from_string (json_str); + json = wp_spa_json_new_wrap_string (json_str); g_assert_nonnull (json); g_assert_false (wp_spa_json_is_unique_owner (json)); @@ -1120,7 +1120,7 @@ test_spa_json_spa_format (void) g_autoptr (WpSpaJson) json = NULL; const gchar json_str[] = "{ name = John age:30, \"car\" null }"; - json = wp_spa_json_new_from_string (json_str); + json = wp_spa_json_new_wrap_string (json_str); g_assert_nonnull (json); g_assert_true (wp_spa_json_is_object (json)); @@ -1202,7 +1202,7 @@ static void test_spa_json_to_string (void) { const gchar json_str[] = "[{\"key0\":\"val0\"}, {\"key1\":\"val1\"}]"; - g_autoptr (WpSpaJson) json = wp_spa_json_new_from_string (json_str); + g_autoptr (WpSpaJson) json = wp_spa_json_new_wrap_string (json_str); g_assert_nonnull (json); {