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 <mschif...@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
>>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
