[OpenWrt-Devel] [LEDE-DEV] [PATCH libubox] blobmsg_json: add new functions blobmsg_format_json_value*

Matthias Schiffer mschiffer at universe-factory.net
Sat Jun 4 11:27:03 EDT 2016


On 06/03/2016 04:55 PM, Eyal Birger wrote:
> 
> Hi,
> 
>> On 3 Jun 2016, at 13:11, Matthias Schiffer <mschiffer at universe-factory.net> wrote:
>>
> (snip)
>>
>> 1) and 2) would allow blobmsg to store everything that json-c can (with the
>> caveat that json-c stores integers as int64 internally, while blobmsg_json
>> uses int32) -
> 
> We also noticed this as a problem for us since when converting json strings to blobmsg, integers become signed and thus no more than INT32_MAX can be used.
> 
> Do you plans to approach this in your patchsets?
> 
> Eyal

I don't think this can be fixed easily without having to adjust all
blobmsg_json users, as the blobmsg_policy entries contain
BLOBMSG_TYPE_INT32 everywhere. I don't know how much the ubus methods are
considered unchangeable ABI.

Possible approaches include:

1) Always map JSON intergers to int64. Will cause an incompatible ABI
change for all ubus calls when used with blobmsg_json.

2) Add new blobmsg_add_json_* functions which use int64. The caller of a
ubus method would need to know if the service excepts int32 or int64
integers, making this more or less unusable for the ubus CLI tool

3) Adjust blobmsg_add_json_* to encode integers as int32 or int64 depending
on the value itself. We'd need to extend the blobmsg_policy with some kind
of BLOBMSG_TYPE_INT which accepts both int32 and int64, and add a
blobmsg_get_int function that can work with different lengths. Existing
software would continue to work as long as the supplied values fit into an
int32.

4) Introduce a new BLOBMSG_TYPE_INT type for variable-length integers,
together with a blobmsg_get_int function (note that, in contrast to 3),
BLOBMSG_TYPE_INT is a real blobmsg type in this approach). The length of
records is encoded in the blobmsg format already. Again, this approach
would need all software to be adjusted.

1) and 4) are very similar and would cause a hard ABI break for many ubus
methods. If we want to avoid a flagday change, 3) seems like the best
option - or some other approach I haven't listed?

Matthias


> 
>> do you think these changes make sense?
>>
>> Would there also be general interest in 3), so it might be integrated into
>> libubox?
>>
>> Regards,
>> Matthias
>>
>>
>>>
>>>>
>>>> Signed-off-by: Matthias Schiffer <mschiffer at universe-factory.net>
>>>> ---
>>>> blobmsg_json.c | 49 ++++++++++++++++++++++++++++++++++++-------------
>>>> blobmsg_json.h | 14 ++++++++++++++
>>>> 2 files changed, 50 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/blobmsg_json.c b/blobmsg_json.c
>>>> index 5713948..538c816 100644
>>>> --- a/blobmsg_json.c
>>>> +++ b/blobmsg_json.c
>>>> @@ -207,7 +207,7 @@ static void blobmsg_format_string(struct strbuf *s, const char *str)
>>>>
>>>> static void blobmsg_format_json_list(struct strbuf *s, struct blob_attr *attr, int len, bool array);
>>>>
>>>> -static void blobmsg_format_element(struct strbuf *s, struct blob_attr *attr, bool array, bool head)
>>>> +static void blobmsg_format_element(struct strbuf *s, struct blob_attr *attr, bool without_name, bool head)
>>>> {
>>>>    const char *data_str;
>>>>    char buf[32];
>>>> @@ -217,7 +217,7 @@ static void blobmsg_format_element(struct strbuf *s, struct blob_attr *attr, boo
>>>>    if (!blobmsg_check_attr(attr, false))
>>>>        return;
>>>>
>>>> -    if (!array && blobmsg_name(attr)[0]) {
>>>> +    if (!without_name && blobmsg_name(attr)[0]) {
>>>>        blobmsg_format_string(s, blobmsg_name(attr));
>>>>        blobmsg_puts(s, ": ", s->indent ? 2 : 1);
>>>>    }
>>>> @@ -286,22 +286,26 @@ static void blobmsg_format_json_list(struct strbuf *s, struct blob_attr *attr, i
>>>>    blobmsg_puts(s, (array ? "]" : "}"), 1);
>>>> }
>>>>
>>>> +static void setup_strbuf(struct strbuf *s, struct blob_attr *attr, blobmsg_json_format_t cb, void *priv, int indent) {
>>>> +    s->len = blob_len(attr);
>>>> +    s->buf = malloc(s->len);
>>>> +    s->pos = 0;
>>>> +    s->custom_format = cb;
>>>> +    s->priv = priv;
>>>> +    s->indent = false;
>>>> +
>>>> +    if (indent >= 0) {
>>>> +        s->indent = true;
>>>> +        s->indent_level = indent;
>>>> +    }
>>>> +}
>>>> +
>>>> char *blobmsg_format_json_with_cb(struct blob_attr *attr, bool list, blobmsg_json_format_t cb, void *priv, int indent)
>>>> {
>>>>    struct strbuf s;
>>>>    bool array;
>>>>
>>>> -    s.len = blob_len(attr);
>>>> -    s.buf = malloc(s.len);
>>>> -    s.pos = 0;
>>>> -    s.custom_format = cb;
>>>> -    s.priv = priv;
>>>> -    s.indent = false;
>>>> -
>>>> -    if (indent >= 0) {
>>>> -        s.indent = true;
>>>> -        s.indent_level = indent;
>>>> -    }
>>>> +    setup_strbuf(&s, attr, cb, priv, indent);
>>>>
>>>>    array = blob_is_extended(attr) &&
>>>>        blobmsg_type(attr) == BLOBMSG_TYPE_ARRAY;
>>>> @@ -321,3 +325,22 @@ char *blobmsg_format_json_with_cb(struct blob_attr *attr, bool list, blobmsg_jso
>>>>
>>>>    return s.buf;
>>>> }
>>>> +
>>>> +char *blobmsg_format_json_value_with_cb(struct blob_attr *attr, blobmsg_json_format_t cb, void *priv, int indent)
>>>> +{
>>>> +    struct strbuf s;
>>>> +
>>>> +    setup_strbuf(&s, attr, cb, priv, indent);
>>>> +
>>>> +    blobmsg_format_element(&s, attr, true, false);
>>>> +
>>>> +    if (!s.len) {
>>>> +        free(s.buf);
>>>> +        return NULL;
>>>> +    }
>>>> +
>>>> +    s.buf = realloc(s.buf, s.pos + 1);
>>>> +    s.buf[s.pos] = 0;
>>>> +
>>>> +    return s.buf;
>>>> +}
>>>> diff --git a/blobmsg_json.h b/blobmsg_json.h
>>>> index cd9ed33..9dfc02d 100644
>>>> --- a/blobmsg_json.h
>>>> +++ b/blobmsg_json.h
>>>> @@ -42,4 +42,18 @@ static inline char *blobmsg_format_json_indent(struct blob_attr *attr, bool list
>>>>    return blobmsg_format_json_with_cb(attr, list, NULL, NULL, indent);
>>>> }
>>>>
>>>> +char *blobmsg_format_json_value_with_cb(struct blob_attr *attr,
>>>> +                    blobmsg_json_format_t cb, void *priv,
>>>> +                    int indent);
>>>> +
>>>> +static inline char *blobmsg_format_json_value(struct blob_attr *attr)
>>>> +{
>>>> +    return blobmsg_format_json_value_with_cb(attr, NULL, NULL, -1);
>>>> +}
>>>> +
>>>> +static inline char *blobmsg_format_json_value_indent(struct blob_attr *attr, int indent)
>>>> +{
>>>> +    return blobmsg_format_json_value_with_cb(attr, NULL, NULL, indent);
>>>> +}
>>>> +
>>>> #endif
>>
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel at lists.openwrt.org
>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20160604/6282a963/attachment.sig>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


More information about the openwrt-devel mailing list