After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 707382 - JSON to GVariant conversion can't convert between strings and numbers
JSON to GVariant conversion can't convert between strings and numbers
Status: RESOLVED FIXED
Product: json-glib
Classification: Core
Component: GObject
git master
Other Linux
: Normal enhancement
: ---
Assigned To: json-glib-maint
json-glib-maint
Depends on:
Blocks:
 
 
Reported: 2013-09-03 13:05 UTC by Joseph Artsimovich
Modified: 2013-12-02 14:38 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Patch implementing string-to-boolean and string-to-number conversions (4.55 KB, patch)
2013-09-03 13:05 UTC, Joseph Artsimovich 		reviewed Details | Review
Updated version of the patch (4.57 KB, patch)
2013-12-02 14:26 UTC, Joseph Artsimovich 		committed Details | Review

Description Joseph Artsimovich 2013-09-03 13:05:37 UTC 
Created attachment 253970 [details] [review]
Patch implementing string-to-boolean and string-to-number conversions

Consider the following JSON: ["123"]
Trying to convert it to GVariant with signature "(i)" fails, as json-glib won't try to convert a string to a number. It would be nice if json-glib could do that.

Let me give a use case:
I am composing a JSON object from several data sources, some of which don't have type info, so I represent those as strings. Then I convert that JSON to GVariant and pass it to a DBus method call. Note that I only get to know DBus method signature at runtime.

I tried to implement arbitrary value-to-value conversion using g_value_transform(), but it turned out g_value_transform() doesn't support string-to-number conversions.

For my purposes, I only need to be able to convert strings to other types. So, I wrote a patch that introduces support for that, without supporting other types of conversions. If that's acceptable, I'd like the patch to be reviewed.
Comment 1 Emmanuele Bassi (:ebassi) 2013-12-02 12:22:27 UTC 
Review of attachment 253970 [details] [review]:

thanks for your patch.

I'm not entirely convinced by this, as it looks like a way to circumvent the type safety of the system, but I don't have a strong objection.

there are a couple of coding style issues to be addressed before pushing the commit upstream.

thanks again!

::: json-glib/json-gvariant.c
@@ +1139,3 @@
     }
 
+  if (JSON_NODE_TYPE(json_node) == JSON_NODE_VALUE &&

coding style: missing space between macro and parenthesis.

@@ +1140,3 @@
 
+  if (JSON_NODE_TYPE(json_node) == JSON_NODE_VALUE &&
+      json_node_get_value_type(json_node) == G_TYPE_STRING)

coding style: missing space between function and parenthesis.

@@ +1158,3 @@
+          variant = gvariant_simple_from_string (str, class, error);
+          goto out;
+        case G_VARIANT_CLASS_UINT16:

coding style: use break with the empty default condition.
Comment 2 Joseph Artsimovich 2013-12-02 14:26:00 UTC 
Created attachment 263301 [details] [review]
Updated version of the patch

Thanks for review. I updated my patch with coding style changes you suggested.
Comment 3 Emmanuele Bassi (:ebassi) 2013-12-02 14:31:17 UTC 
Review of attachment 263301 [details] [review]:

okay.
Comment 4 Emmanuele Bassi (:ebassi) 2013-12-02 14:38:00 UTC 
attachment 263301 [details] [review] pushed to master