Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wryun opened this issue Oct 26, 2023 · 1 comment

Comments

@wryun
Copy link

wryun commented Oct 26, 2023

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-
Copy link
Contributor

jow- commented Oct 26, 2023

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:

  • BLOBMSG_NUMBERS_PREFER_DOUBLE - Convert both json_type_int and json_type_double to BLOBMSG_TYPE_DOUBLE
  • BLOBMSG_NUMBERS_PREFER_INT64 - Convert json_type_int and integral json_type_double to BLOBMSG_TYPE_INT64, fractional JSON doubles to BLOBMSG_TYPE_DOUBLE
  • BLOBMSG_NUMBERS_PREFER_INT32 - Convert json_type_int [-2147483647..2147483647] to BLOBMSG_TYPE_INT32, smaller/larger json_type_int to BLOBMSG_TYPE_INT64, integral json_type_double to BLOBMSG_TYPE_INT32 (or BLOBMSG_TYPE_INT64 if it exceeds range) and other doubles to BLOBMSG_TYPE_DOUBLE
  • BLOBMSG_NUMBERS_FORCE_INT32 - Like above but truncate values outside of the int32_t range

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants