| Message ID | 20161019101034.20538-1-fl@n621.de |
|---|---|
| State | Changes Requested |
| Delegated to: | John Crispin |
| Headers | show |
Hi, On Wed, Oct 19, 2016 at 1:10 PM, Florian Larysch <fl@n621.de> wrote: > Currently, libubox uses json_object_get_int when converting a JSON > document into a blobmsg. However, json-c stores integers as int64_t > values and may clamp the value when asked to return it as an int32_t. > > Always use json_object_get_int64 and use a u32/u64 blobmsg value > depending on the magnitude of the returned value. This has been somewhat discussed in the past (see https://lists.openwrt.org/pipermail/openwrt-devel/2016-June/041700.html). IIUC, your change makes the type of fields vary upon their values. I'm wondering how you suggest applications parse the resulting blob messages? Eyal.
Hi, > IIUC, your change makes the type of fields vary upon their values. > I'm wondering how you suggest applications parse the resulting blob messages? you're right, this doesn't make sense. In my case, the only consumer was using blobmsg_format_json(), which handles element types dynamically. This patch would break users of blobmsg_parse() in some cases, please disregard it. > This has been somewhat discussed in the past (see > https://lists.openwrt.org/pipermail/openwrt-devel/2016-June/041700.html). While I agree with you that all options would cause a ABI break in the strict sense that the raw blob might change in an incompatible way, this could be handled in libubox without causing an ABI break there (for example by capping values like libjson-c does). In that light, options 3 and 4 are largely equivalent. Options 1 and 2 would be a bit wasteful, IMO. Florian
diff --git a/blobmsg_json.c b/blobmsg_json.c index 8a6ed8f..1ae4954 100644 --- a/blobmsg_json.c +++ b/blobmsg_json.c @@ -47,6 +47,7 @@ static bool blobmsg_add_array(struct blob_buf *b, struct array_list *a) bool blobmsg_add_json_element(struct blob_buf *b, const char *name, json_object *obj) { bool ret = true; + int64_t n; void *c; switch (json_object_get_type(obj)) { @@ -67,7 +68,13 @@ bool blobmsg_add_json_element(struct blob_buf *b, const char *name, json_object blobmsg_add_u8(b, name, json_object_get_boolean(obj)); break; case json_type_int: - blobmsg_add_u32(b, name, json_object_get_int(obj)); + n = json_object_get_int64(obj); + + if (n >= INT32_MIN && n <= INT32_MAX) + blobmsg_add_u32(b, name, n); + else + blobmsg_add_u64(b, name, n); + break; case json_type_null: blobmsg_add_field(b, BLOBMSG_TYPE_UNSPEC, name, NULL, 0);
Currently, libubox uses json_object_get_int when converting a JSON document into a blobmsg. However, json-c stores integers as int64_t values and may clamp the value when asked to return it as an int32_t. Always use json_object_get_int64 and use a u32/u64 blobmsg value depending on the magnitude of the returned value. Signed-off-by: Florian Larysch <fl@n621.de> --- blobmsg_json.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)