[OpenWrt-Devel,libubox] blobmsg_json: add new functions blobmsg_format_json_value*

The current blobmsg_format_json* functions will return invalid JSON when
the "list" argument is given as false (blobmsg_format_element() will
output the name of the blob_attr as if the value is printed as part of a
JSON object).

To avoid breaking software relying on this behaviour, introduce new
functions which will never print the blob_attr name and thus always
produce valid JSON.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---
 blobmsg_json.c | 49 ++++++++++++++++++++++++++++++++++++-------------
 blobmsg_json.h | 14 ++++++++++++++
 2 files changed, 50 insertions(+), 13 deletions(-)

On 01/06/2016 22:27, Matthias Schiffer wrote:
> The current blobmsg_format_json* functions will return invalid JSON when
> the "list" argument is given as false (blobmsg_format_element() will
> output the name of the blob_attr as if the value is printed as part of a
> JSON object).
> 
> To avoid breaking software relying on this behaviour, introduce new
> functions which will never print the blob_attr name and thus always
> produce valid JSON.

what software relies on this function/behaviour ? maybe we should mark
the current implementation as deprected and drop support in time X.
producing broken json syntax seems very fragile.

	John

> 
> Signed-off-by: Matthias Schiffer <mschiffer@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
>

On 06/03/2016 11:05 AM, John Crispin wrote:
> 
> 
> On 01/06/2016 22:27, Matthias Schiffer wrote:
>> The current blobmsg_format_json* functions will return invalid JSON when
>> the "list" argument is given as false (blobmsg_format_element() will
>> output the name of the blob_attr as if the value is printed as part of a
>> JSON object).
>>
>> To avoid breaking software relying on this behaviour, introduce new
>> functions which will never print the blob_attr name and thus always
>> produce valid JSON.
> 
> what software relies on this function/behaviour ? maybe we should mark
> the current implementation as deprected and drop support in time X.
> producing broken json syntax seems very fragile.
> 
> 	John

The most promiment usage is the output of `ubus list -v`, most other uses
seem to be just error messages.

I've found a number of other issues in blobmsg_json (more broken JSON
output, missing or broken allocation failure handling, ...). You can drop
this patch for now, I'll send a new version together with some other
patches next week.

Three more remarks/questions:

1) I'd like to add two more blobmsg datatypes, F32 and F64 for
single/double precision floating-point numbers, to complete the
JSON<->blobmsg mapping. I've already looked into possible platform-specific
encoding issues of floats, the only platform-specific part seems to be some
NaN encodings, which would need to be mapped to a generic NaN encoding.
(NaN is not valid JSON, so it wouldn't be important for blobmsg_json
anyways; json-c accepts NaN and +/- Infinity though).
Usecase: I'm currently implementing a configuration storage daemon as part
of my GSoC project. In particular, I think that geocoordinates would be
nice to store as floats.

2) blogmsg currently doesn't allow tables with empty keys. In JSON, the
empty string is not special and can be used as a key. I'd like to adjust
blobmsg to allow this.

3) Another thing I'm working on is a blobtree library. blobtrees are very
similar to blobmsg, but tables and arrays store just a list of pointers to
the child blobs. This allows efficient manipulation of such trees (for the
mentioned configuration storage daemon), while still making conversion from
and to blobmsg as simple as possible.

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) - 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@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
>>

Hi,

> On 3 Jun 2016, at 13:11, Matthias Schiffer <mschiffer@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

> 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@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@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

On 06/03/2016 04:55 PM, Eyal Birger wrote:
> 
> Hi,
> 
>> On 3 Jun 2016, at 13:11, Matthias Schiffer <mschiffer@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@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@lists.openwrt.org
>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

On Sat, Jun 4, 2016 at 6:27 PM, Matthias Schiffer
<mschiffer@universe-factory.net> wrote:
> On 06/03/2016 04:55 PM, Eyal Birger wrote:
>>
>> Hi,
>>
>>> On 3 Jun 2016, at 13:11, Matthias Schiffer <mschiffer@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.
>

Alas, I think this may still cause an ABI change, as the current int32
implementation caps
values at INT32_MAX; so when BLOBMSG_TYPE_INT32 is specified in the
policy, a value
is _always_ available, whereas in this solution, sometimes it won't
be. Am I wrong?

> 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?
>

I tried to think of a way in which we can have blobmsg_format_element() encode
*both* int32 and int64 values at the same time. It might work for parsing,
but won't work with the current serialization code, not to mention
that it would be
a very ugly hack.

Eyal.

> Matthias