openwrt / libubox

[MIRROR] C utility functions for OpenWrt
https://git.openwrt.org/?p=project/libubox.git;
14 stars 14 forks source link

JSON -> Blob conversion makes number parsing hard, particularly for doubles #8

Open wryun opened 1 year ago

wryun commented 1 year ago

The root issue is that Blobs' have a lot of numeric types and JSON only has one. The JSON-C library attempts to guess the type and this guess is reflected in the blob format:

    case json_type_int: {
        int64_t i64 = json_object_get_int64(obj);
        if (i64 >= INT32_MIN && i64 <= INT32_MAX) {
            blobmsg_add_u32(b, name, (uint32_t)i64);
        } else {
            blobmsg_add_u64(b, name, (uint64_t)i64);
        }
        break;
    }
    case json_type_double:
        blobmsg_add_double(b, name, json_object_get_double(obj));
        break;

However, consider the case of passing a double in via JSON. The upstream code might pass something like 2.1, or it might pass something like 2, noting that JSON/Javascript thinks of all things as Numbers; the former will be turned into a BLOBMSG_TYPE_DOUBLE while the latter will be turned into a BLOBMSG_TYPE_INT32. As far as I understand it, the only way to handle this with a policy when parsing the resulting message is to use BLOBMSG_TYPE_UNSPEC and then write your own conversion. e.g.

static inline double blobmsg_cast_double(struct blob_attr *attr)
{
    double tmp = 0;

    if (blobmsg_type(attr) == BLOBMSG_TYPE_INT64)
        tmp = blobmsg_get_u64(attr);
    else if (blobmsg_type(attr) == BLOBMSG_TYPE_INT32)
        tmp = (int32_t)blobmsg_get_u32(attr);
    else if (blobmsg_type(attr) == BLOBMSG_TYPE_INT16)
        tmp = (int16_t)blobmsg_get_u16(attr);
    else if (blobmsg_type(attr) == BLOBMSG_TYPE_INT8)
        tmp = (int8_t)blobmsg_get_u8(attr);
    else if (blobmsg_type(attr) == BLOBMSG_TYPE_DOUBLE)
        tmp = blobmsg_get_double(attr);

    return tmp;
}

This whole situation makes me a little uncomfortable. My preference would be for all JSON numbers to be represented as doubles in the blobmsg, but this is hardly backwards compatible. A conservative approach would be to add a BLOBMSG_CAST_DOUBLE type (cf BLOBMSG_CAST_INT64) and the helper functions above. I'd also like there to be equivalent double->int casting functions, potentially changing the behaviour of BLOBMSG_CAST_INT64 to support doubles.

Thoughts?

jow- commented 1 year ago

I suppose you refer to blobmsg_add_json_element() ?

I would propose to implement a new API bool blobmsg_add_json_element_ex(struct blob_buf *b, const char *name, struct json_object *obj, unsigned int flags) which accepts a number of flag such as:

Adding a cast mechanism analogous to the existing BLOBMSG_CAST_INT64 machinery makes sense to me.

Treating anything numeric as double internally, while being more in line with JSON's idea of numeric types, sounds not very resource efficient wrt. embedded system usage.