* Re: [PATCH v2 2/3] json: introduce JSON module
@ 2021-12-09 19:23 Denis Kenzior
0 siblings, 0 replies; 4+ messages in thread
From: Denis Kenzior @ 2021-12-09 19:23 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 2705 bytes --]
Hi James,
>> Have you considered using an intermediate object to hold the bits
>> that need to
>> be opaque? That way json_iter objects could be public and held on
>> the stack.
>> Would make things easier to use / simpler to implement.
>>
>> So roughly:
>>
>> struct json_contents;
>>
>> struct json_iter {
>> struct json_contents *contents;
>> int begin;
>> int end;
>> };
>
> This didn't feel consistent with any other iterators on cleanup:
>
> struct json_iter iter;
>
> json_iter_init(&iter, json, json_len);
>
> ...
>
> json_iter_free(&iter);
>
Well, you would do something like:
struct json_contents *c;
struct json_iter iter;
struct json_iter nested;
_auto_(l_free) char *foo = NULL;
c = json_contents_new(json, json_len);
json_iter_init(&iter, c);
json_iter_parse(&iter, JSON_STRING, "foo",
&foo, JSON_FLAG_MANDATORY,
JSON_OBJECT, "bar",
&nested, JSON_FLAG_OPTIONAL,
JSON_UNDEFINED);
json_iter_parse(&nested, ...);
json_contents_free(c);
>>
>> Do you need jsmn_init here?
>
> Yes, below.
Ah right, sorry for the noise.
>>
>> So value->size == 0 implies a basic type?
>
> AFAIK any value that is not an object will have a size of zero, so this
> could include strings as well not just basic types.
So essentially anything that is not a container? integers, strings, etc.
>>
>>> + > + for (i = 0; i < value->size; i++)
>>> + value = next_key(iter, value);
>>
>> So this should be recursively skipping tokens when value is non-
>> basic. The
>> naming is a bit confusing here
>>
>>> +
>>> + return value;
>>
>> Since you're returning 'value', but the reader expects a 'key'.
>
> Yes, some re-naming is in order. This is actually returning a key
> though, I just reused 'value' rather than a new variable. Something
> like 'token' or 'tmp' would be better.
Right, this is doing the right thing, just nitpicking the variable naming.
>> May not be as big of a deal in this particular case, but this
>> approach is
>> side-effecting on failure. You could parse the object part of the
>> way through,
>> then fail to find a key and bail, but you may have assigned stuff
>> along the way.
>
> Yeah I could not think of a decent way to handle this without iterating
> twice (verification first, then assignment) since we lose reference to
> the previous set of arguments with each iteration.
You could iterate over the va_arg list, build a temporary structure and perform
the parse. If the parse succeeds, then assign the values in the final step.
Regards,
-Denis
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/3] json: introduce JSON module
@ 2021-12-09 19:10 James Prestwood
0 siblings, 0 replies; 4+ messages in thread
From: James Prestwood @ 2021-12-09 19:10 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 19038 bytes --]
On Thu, 2021-12-09 at 11:52 -0600, Denis Kenzior wrote:
> Hi James,
>
> On 12/8/21 3:36 PM, James Prestwood wrote:
> > This is a minimal wrapper around jsmn.h to make things a bit easier
> > for iterating through a JSON object.
> >
> > An iterator can be created/destroyed with json_iter_{new,free} and
> > values can be parsed from this iterator using json_iter_parse.
> >
>
> Have you considered using an intermediate object to hold the bits
> that need to
> be opaque? That way json_iter objects could be public and held on
> the stack.
> Would make things easier to use / simpler to implement.
>
> So roughly:
>
> struct json_contents;
>
> struct json_iter {
> struct json_contents *contents;
> int begin;
> int end;
> };
This didn't feel consistent with any other iterators on cleanup:
struct json_iter iter;
json_iter_init(&iter, json, json_len);
...
json_iter_free(&iter);
>
> also you could possibly track parent objects if needed.
>
> > Arguments to json_iter_parse should use
> > JSON_MANDATORY/JSON_OPTIONAL
> > macros, and terminate the argument list with JSON_FLAG_INVALID.
> >
> > The value pointer in the MANDATORY/OPTIONAL macros can be NULL, in
> > which case the presence of the key/value is checked but no data is
> > returned out.
> >
> > Currently only JSON_STRING and JSON_OBJECT types are supported.
> > String
> > values should be of type struct iovec *, and objects should be of
>
> Ok, I wonder if we could allocate the strings instead of having the
> caller do this.
Sure. Initially this was supposed to be a minimal wrapper and the
allocation seemed like too much, but this has evolved :)
>
> > type struct json_iter **. These nested objects can be parsed again
> > using json_iter_parse and should not be freed (the original
> > iterator
> > keeps track of nested object iterators).
>
> It would be nice if these were simply on the stack to cut down the
> queue management.
>
> > ---
> > Makefile.am | 1 +
> > src/json.c | 222
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > src/json.h | 84 ++++++++++++++++++++
> > 3 files changed, 307 insertions(+)
> > create mode 100644 src/json.c
> > create mode 100644 src/json.h
> >
> > v2:
> > * Re-wrote to remove callback style parsing. Instead the expected
> > values
> > are provided to the parser directly.
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 7fe4b5c3..1ae7e3c0 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -249,6 +249,7 @@ src_iwd_SOURCES = src/main.c linux/nl80211.h
> > src/iwd.h src/missing.h \
> > src/sysfs.h src/sysfs.c \
> > src/offchannel.h
> > src/offchannel.c \
> > src/dpp-util.h src/dpp-
> > util.c \
> > + src/json.h src/json.c \
> > $(eap_sources) \
> > $(builtin_sources)
> >
> > diff --git a/src/json.c b/src/json.c
> > new file mode 100644
> > index 00000000..9f6c744f
> > --- /dev/null
> > +++ b/src/json.c
> > @@ -0,0 +1,222 @@
> > +/*
> > + *
> > + * Wireless daemon for Linux
> > + *
> > + * Copyright (C) 2013-2019 Intel Corporation. All rights
> > reserved.
>
> Please update this
>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later
> > version.
> > + *
> > + * This library is distributed in the hope that it will be
> > useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General
> > Public
> > + * License along with this library; if not, write to the Free
> > Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > 02110-1301 USA
> > + *
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <errno.h>
> > +
> > +#include <ell/ell.h>
> > +
> > +#include "src/json.h"
> > +
> > +#include "shared/jsmn.h"
> > +
> > +/* Max number of tokens supported. Increase if larger objects are
> > expected */
> > +#define JSON_DEFAULT_TOKENS 30
> > +
> > +#define TOK_LEN(token) (token)->end - (token)->start
> > +#define TOK_PTR(json, token) (void *)((json) + (token)->start)
> > +
> > +struct json_iter {
> > + const char *json;
> > + size_t json_len;
> > + jsmntok_t *tokens;
> > + int tokens_len;
> > + jsmn_parser *p;
> > + struct l_queue *children;
> > + bool root : 1;
> > +};
> > +
> > +static struct json_iter *iter_recurse(struct json_iter *parent,
> > + jsmntok_t *object)
> > +{
> > + struct json_iter *iter = l_new(struct json_iter, 1);
> > +
> > + iter->json = parent->json;
> > + iter->json_len = parent->json_len;
> > + iter->tokens = object;
> > + iter->children = l_queue_new();
> > +
> > + /*
> > + * Since object->size only indicates the number of keys in
> > an object
> > + * we cannot easily calculate the number of tokens
> > remaining (for this
> > + * object) since each object may have nested objects. Set
> > tokens_len to
> > + * the number of tokens remaining in the overall list after
> > 'object'
>
> This seems brittle. Can we recursively count the number of tokens
> for each child?
So this is really only for bounds checking, which may not actually be
requred as that comment below suggests. I think so long as we trust
JSMN we can guarantee its parsed tokens will always contain key/value
pairs. Still, a maximum bounds probably doesn't hurt. So yes we can
limit the bounds of these recursive iterators to only the child object.
>
> > + */
> > + iter->tokens_len = parent->tokens_len -
> > + ((object - parent->tokens) %
> > sizeof(jsmntok_t));
> > +
> > + l_queue_push_head(parent->children, iter);
> > +
> > + return iter;
> > +}
> > +
> > +struct json_iter *json_iter_new(const char *json, size_t json_len)
> > +{
> > + struct json_iter *iter = l_new(struct json_iter, 1);
> > +
> > + iter->json = json;
> > + iter->json_len = json_len;
> > + iter->p = l_new(jsmn_parser, 1);
>
> Do you need jsmn_init here?
Yes, below.
>
> > + iter->tokens = l_new(jsmntok_t, JSON_DEFAULT_TOKENS);
> > + iter->root = true;
> > +
> > + jsmn_init(iter->p);
> > + iter->tokens_len = jsmn_parse(iter->p, iter->json, iter-
> > >json_len,
> > + iter->tokens,
> > JSON_DEFAULT_TOKENS);
> > + if (iter->tokens_len < 0) {
> > + json_iter_free(iter);
> > + return NULL;
> > + }
> > +
> > + iter->children = l_queue_new();
> > +
> > + return iter;
> > +}
> > +
> > +static void json_iter_destroy(void *data)
> > +{
> > + struct json_iter *iter = data;
> > +
> > + json_iter_free(iter);
> > +}
> > +
> > +void json_iter_free(struct json_iter *iter)
> > +{
> > + if (iter->root) {
> > + l_free(iter->p);
> > + l_free(iter->tokens);
> > + }
> > +
> > + l_queue_destroy(iter->children, json_iter_destroy);
>
> You could do:
> l_queue_destroy(iter->children,
> (l_queue_destroy_func_t *) json_iter_free);
>
> > + l_free(iter);
> > +}
> > +
> > +static jsmntok_t *next_key(struct json_iter *iter, jsmntok_t
> > *current)
> > +{
> > + jsmntok_t *value;
> > + int i;
> > +
> > + /*
> > + * In theory JSMN should prevent any of these bounds checks
> > from failing
>
> Not very reassuring :)
This was more paranoia than anything.
>
> > + */
> > + if (current + 1 > iter->tokens + iter->tokens_len)
> > + return NULL;
> > +
> > + value = current + 1;
> > +
> > + if (value->size == 0)
> > + return value + 1;
>
> So value->size == 0 implies a basic type?
AFAIK any value that is not an object will have a size of zero, so this
could include strings as well not just basic types.
>
> > + > + for (i = 0; i < value->size; i++)
> > + value = next_key(iter, value);
>
> So this should be recursively skipping tokens when value is non-
> basic. The
> naming is a bit confusing here
>
> > +
> > + return value;
>
> Since you're returning 'value', but the reader expects a 'key'.
Yes, some re-naming is in order. This is actually returning a key
though, I just reused 'value' rather than a new variable. Something
like 'token' or 'tmp' would be better.
>
> > +}
> > +
> > +/*
> > + * Checks that the keys value (t + 1) is in bounds, the value type
> > matches,
> > + * key length matches, and key string match
> > + */
> > +#define TOKEN_MATCHES(iter, t, key, type) \
> > + (t) + 1 < iter->tokens + iter->tokens_len && \
> > + (jsmntype_t)(type) == ((t) + 1)->type && \
> > + TOK_LEN((t)) == (int)strlen((key)) && \
> > + !memcmp(TOK_PTR((iter)->json, (t)), (key), TOK_LEN((t)))
> > +
> > +bool json_iter_parse(struct json_iter *iter, ...)
> > +{
> > + va_list va;
> > + int i;
> > + int num = iter->tokens->size;
> > + jsmntok_t *next;
> > + struct iovec *sval;
> > + struct json_iter **oval;
> > +
> > + /* Empty object {} */
> > + if (iter->tokens_len < 1)
> > + return false;
> > +
> > + va_start(va, iter);
> > +
> > + while (true) {
> > + enum json_flag flag;
> > + char *key;
> > + enum json_type type;
> > + void *value;
> > + jsmntok_t *v = NULL;
> > +
> > + flag = va_arg(va, enum json_flag);
> > +
> > + if (flag == JSON_FLAG_INVALID)
> > + break;
> > +
> > + key = va_arg(va, char *);
> > + type = va_arg(va, enum json_type);
> > + value = va_arg(va, void *);
> > +
> > + next = iter->tokens + 1;
> > +
> > + /* Iterate over this objects keys */
> > + for (i = 0; i < num; i++) {
> > + if (TOKEN_MATCHES(iter, next, key, type)) {
> > + v = next + 1;
> > + break;
>
> What if the key matches but the type doesn't? Shouldn't we bail
> early?
Yep, I'll add some extra checks.
>
> > + }
> > +
> > + next = next_key(iter, next);
> > + if (!next)
> > + return false;
>
> May not be as big of a deal in this particular case, but this
> approach is
> side-effecting on failure. You could parse the object part of the
> way through,
> then fail to find a key and bail, but you may have assigned stuff
> along the way.
Yeah I could not think of a decent way to handle this without iterating
twice (verification first, then assignment) since we lose reference to
the previous set of arguments with each iteration.
>
> > + }
> > +
> > + if (flag == JSON_FLAG_MANDATORY && !v)
> > + return false;
> > +
> > + switch (type) {
> > + case JSON_STRING:
> > + if (!value)
> > + continue;
> > +
> > + sval = value;
> > + sval->iov_base = v ? TOK_PTR(iter->json, v)
> > : NULL;
> > + sval->iov_len = v ? TOK_LEN(v) : 0;
> > +
> > + break;
> > + case JSON_OBJECT:
> > + if (!value)
> > + continue;
> > +
> > + oval = value;
> > + *oval = v ? iter_recurse(iter, v) : NULL;
> > +
> > + break;
> > + default:
> > + return false;
> > + }
> > + }
> > +
> > + va_end(va);
> > +
> > + return true;
> > +}
> > diff --git a/src/json.h b/src/json.h
> > new file mode 100644
> > index 00000000..202445ee
> > --- /dev/null
> > +++ b/src/json.h
> > @@ -0,0 +1,84 @@
> > +/*
> > + *
> > + * Wireless daemon for Linux
> > + *
> > + * Copyright (C) 2021 Intel Corporation. All rights reserved.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later
> > version.
> > + *
> > + * This library is distributed in the hope that it will be
> > useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General
> > Public
> > + * License along with this library; if not, write to the Free
> > Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > 02110-1301 USA
> > + *
> > + */
> > +
> > +struct json_iter;
> > +
> > +/*
> > + * Identical to JSMN types
> > + */
> > +enum json_type {
> > + JSON_UNDEFINED = 0,
> > + JSON_OBJECT = 1 << 0,
> > + JSON_ARRAY = 1 << 1,
> > + JSON_STRING = 1 << 2,
> > + JSON_PRIMITIVE = 1 << 3
> > +};
> > +
> > +enum json_flag {
> > + JSON_FLAG_INVALID = 0,
> > + JSON_FLAG_MANDATORY = 1,
> > + JSON_FLAG_OPTIONAL = 2,
> > +};
> > +
> > +#define JSON_MANDATORY(key, type, out) \
> > + JSON_FLAG_MANDATORY, (key), (type), (out)
> > +
> > +#define JSON_OPTIONAL(key, type, out) \
> > + JSON_FLAG_OPTIONAL, (key), (type), (out)
> > +
> > +#define JSON_STR_EQ(iov, s) \
> > + ((iov)->iov_len == strlen((s)) && \
> > + !memcmp((iov)->iov_base, (s), (iov)->iov_len))
> > +
> > +#define JSON_TO_STR(iov) \
> > +({ \
> > + char *_tmp = l_malloc((iov)->iov_len + 1); \
> > + memcpy(_tmp, (iov)->iov_base, (iov)->iov_len); \
> > + _tmp[(iov)->iov_len] = '\0'; \
> > + _tmp; \
> > +})
> > +
> > +struct json_iter *json_iter_new(const char *json, size_t
> > json_len);
> > +void json_iter_free(struct json_iter *iter);
> > +
> > +/*
> > + * Parse an arbitrary number of key/value pairs from a JSON
> > iterator. Initially
> > + * a new JSON iterator should be created with json_iter_new() and,
> > when done,
> > + * freed with json_iter_free. Nested objects are also parsed with
> > this function
> > + * but these iterators should not be freed and are tracked by the
> > original
> > + * iterator.
> > + *
> > + * Arguments should be specified using JSON_MANDATORY or
> > JSON_OPTIONAL:
> > + *
> > + * r = json_iter_parse(iter, JSON_MANDATORY("mykey", JSON_STRING,
> > &strvalue),
> > + * JSON_OPTIONAL("optkey", JSON_STRING,
> > &optvalue));
> > + *
> > + * String values should be of type struct iovec *
> > + * Object values should be of type struct json_iter **
> > + *
> > + * No other types are supported at this time, and json_iter_parse
> > will fail if
> > + * other types are encountered.
> > + *
> > + * JSON_OPTIONAL string values will have a NULL/0 iov_base/iov_len
> > if they were
> > + * not found. JSON_OPTIONAL object value iterators will be NULL if
> > not found.
> > + */
> > +bool json_iter_parse(struct json_iter *iter, ...);
>
> Just a nit, but it is a bit clearer to start the '...' after either a
> printf-style string or an argument that could carry the sentintel
> value. For
> example:
>
> int nl80211_parse_attrs(struct l_genl_msg *msg, int tag, ...);
>
> It isn't much, but gives just a bit extra context for how the API is
> supposed to
> be used. So in the case of json_iter_parse it would be enum
> json_flag.
>
> Though really, I wonder why json_iter_parse doesn't use the sequence
> of:
>
> (json_type, key, value, flags) ?
Yeah this is fine, I can reorder them like this and add a new
JSON_INVALID type to signal the end.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/3] json: introduce JSON module
@ 2021-12-09 17:52 Denis Kenzior
0 siblings, 0 replies; 4+ messages in thread
From: Denis Kenzior @ 2021-12-09 17:52 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 12854 bytes --]
Hi James,
On 12/8/21 3:36 PM, James Prestwood wrote:
> This is a minimal wrapper around jsmn.h to make things a bit easier
> for iterating through a JSON object.
>
> An iterator can be created/destroyed with json_iter_{new,free} and
> values can be parsed from this iterator using json_iter_parse.
>
Have you considered using an intermediate object to hold the bits that need to
be opaque? That way json_iter objects could be public and held on the stack.
Would make things easier to use / simpler to implement.
So roughly:
struct json_contents;
struct json_iter {
struct json_contents *contents;
int begin;
int end;
};
also you could possibly track parent objects if needed.
> Arguments to json_iter_parse should use JSON_MANDATORY/JSON_OPTIONAL
> macros, and terminate the argument list with JSON_FLAG_INVALID.
>
> The value pointer in the MANDATORY/OPTIONAL macros can be NULL, in
> which case the presence of the key/value is checked but no data is
> returned out.
>
> Currently only JSON_STRING and JSON_OBJECT types are supported. String
> values should be of type struct iovec *, and objects should be of
Ok, I wonder if we could allocate the strings instead of having the caller do this.
> type struct json_iter **. These nested objects can be parsed again
> using json_iter_parse and should not be freed (the original iterator
> keeps track of nested object iterators).
It would be nice if these were simply on the stack to cut down the queue management.
> ---
> Makefile.am | 1 +
> src/json.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/json.h | 84 ++++++++++++++++++++
> 3 files changed, 307 insertions(+)
> create mode 100644 src/json.c
> create mode 100644 src/json.h
>
> v2:
> * Re-wrote to remove callback style parsing. Instead the expected values
> are provided to the parser directly.
>
> diff --git a/Makefile.am b/Makefile.am
> index 7fe4b5c3..1ae7e3c0 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -249,6 +249,7 @@ src_iwd_SOURCES = src/main.c linux/nl80211.h src/iwd.h src/missing.h \
> src/sysfs.h src/sysfs.c \
> src/offchannel.h src/offchannel.c \
> src/dpp-util.h src/dpp-util.c \
> + src/json.h src/json.c \
> $(eap_sources) \
> $(builtin_sources)
>
> diff --git a/src/json.c b/src/json.c
> new file mode 100644
> index 00000000..9f6c744f
> --- /dev/null
> +++ b/src/json.c
> @@ -0,0 +1,222 @@
> +/*
> + *
> + * Wireless daemon for Linux
> + *
> + * Copyright (C) 2013-2019 Intel Corporation. All rights reserved.
Please update this
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <errno.h>
> +
> +#include <ell/ell.h>
> +
> +#include "src/json.h"
> +
> +#include "shared/jsmn.h"
> +
> +/* Max number of tokens supported. Increase if larger objects are expected */
> +#define JSON_DEFAULT_TOKENS 30
> +
> +#define TOK_LEN(token) (token)->end - (token)->start
> +#define TOK_PTR(json, token) (void *)((json) + (token)->start)
> +
> +struct json_iter {
> + const char *json;
> + size_t json_len;
> + jsmntok_t *tokens;
> + int tokens_len;
> + jsmn_parser *p;
> + struct l_queue *children;
> + bool root : 1;
> +};
> +
> +static struct json_iter *iter_recurse(struct json_iter *parent,
> + jsmntok_t *object)
> +{
> + struct json_iter *iter = l_new(struct json_iter, 1);
> +
> + iter->json = parent->json;
> + iter->json_len = parent->json_len;
> + iter->tokens = object;
> + iter->children = l_queue_new();
> +
> + /*
> + * Since object->size only indicates the number of keys in an object
> + * we cannot easily calculate the number of tokens remaining (for this
> + * object) since each object may have nested objects. Set tokens_len to
> + * the number of tokens remaining in the overall list after 'object'
This seems brittle. Can we recursively count the number of tokens for each child?
> + */
> + iter->tokens_len = parent->tokens_len -
> + ((object - parent->tokens) % sizeof(jsmntok_t));
> +
> + l_queue_push_head(parent->children, iter);
> +
> + return iter;
> +}
> +
> +struct json_iter *json_iter_new(const char *json, size_t json_len)
> +{
> + struct json_iter *iter = l_new(struct json_iter, 1);
> +
> + iter->json = json;
> + iter->json_len = json_len;
> + iter->p = l_new(jsmn_parser, 1);
Do you need jsmn_init here?
> + iter->tokens = l_new(jsmntok_t, JSON_DEFAULT_TOKENS);
> + iter->root = true;
> +
> + jsmn_init(iter->p);
> + iter->tokens_len = jsmn_parse(iter->p, iter->json, iter->json_len,
> + iter->tokens, JSON_DEFAULT_TOKENS);
> + if (iter->tokens_len < 0) {
> + json_iter_free(iter);
> + return NULL;
> + }
> +
> + iter->children = l_queue_new();
> +
> + return iter;
> +}
> +
> +static void json_iter_destroy(void *data)
> +{
> + struct json_iter *iter = data;
> +
> + json_iter_free(iter);
> +}
> +
> +void json_iter_free(struct json_iter *iter)
> +{
> + if (iter->root) {
> + l_free(iter->p);
> + l_free(iter->tokens);
> + }
> +
> + l_queue_destroy(iter->children, json_iter_destroy);
You could do:
l_queue_destroy(iter->children,
(l_queue_destroy_func_t *) json_iter_free);
> + l_free(iter);
> +}
> +
> +static jsmntok_t *next_key(struct json_iter *iter, jsmntok_t *current)
> +{
> + jsmntok_t *value;
> + int i;
> +
> + /*
> + * In theory JSMN should prevent any of these bounds checks from failing
Not very reassuring :)
> + */
> + if (current + 1 > iter->tokens + iter->tokens_len)
> + return NULL;
> +
> + value = current + 1;
> +
> + if (value->size == 0)
> + return value + 1;
So value->size == 0 implies a basic type?
> + > + for (i = 0; i < value->size; i++)
> + value = next_key(iter, value);
So this should be recursively skipping tokens when value is non-basic. The
naming is a bit confusing here
> +
> + return value;
Since you're returning 'value', but the reader expects a 'key'.
> +}
> +
> +/*
> + * Checks that the keys value (t + 1) is in bounds, the value type matches,
> + * key length matches, and key string match
> + */
> +#define TOKEN_MATCHES(iter, t, key, type) \
> + (t) + 1 < iter->tokens + iter->tokens_len && \
> + (jsmntype_t)(type) == ((t) + 1)->type && \
> + TOK_LEN((t)) == (int)strlen((key)) && \
> + !memcmp(TOK_PTR((iter)->json, (t)), (key), TOK_LEN((t)))
> +
> +bool json_iter_parse(struct json_iter *iter, ...)
> +{
> + va_list va;
> + int i;
> + int num = iter->tokens->size;
> + jsmntok_t *next;
> + struct iovec *sval;
> + struct json_iter **oval;
> +
> + /* Empty object {} */
> + if (iter->tokens_len < 1)
> + return false;
> +
> + va_start(va, iter);
> +
> + while (true) {
> + enum json_flag flag;
> + char *key;
> + enum json_type type;
> + void *value;
> + jsmntok_t *v = NULL;
> +
> + flag = va_arg(va, enum json_flag);
> +
> + if (flag == JSON_FLAG_INVALID)
> + break;
> +
> + key = va_arg(va, char *);
> + type = va_arg(va, enum json_type);
> + value = va_arg(va, void *);
> +
> + next = iter->tokens + 1;
> +
> + /* Iterate over this objects keys */
> + for (i = 0; i < num; i++) {
> + if (TOKEN_MATCHES(iter, next, key, type)) {
> + v = next + 1;
> + break;
What if the key matches but the type doesn't? Shouldn't we bail early?
> + }
> +
> + next = next_key(iter, next);
> + if (!next)
> + return false;
May not be as big of a deal in this particular case, but this approach is
side-effecting on failure. You could parse the object part of the way through,
then fail to find a key and bail, but you may have assigned stuff along the way.
> + }
> +
> + if (flag == JSON_FLAG_MANDATORY && !v)
> + return false;
> +
> + switch (type) {
> + case JSON_STRING:
> + if (!value)
> + continue;
> +
> + sval = value;
> + sval->iov_base = v ? TOK_PTR(iter->json, v) : NULL;
> + sval->iov_len = v ? TOK_LEN(v) : 0;
> +
> + break;
> + case JSON_OBJECT:
> + if (!value)
> + continue;
> +
> + oval = value;
> + *oval = v ? iter_recurse(iter, v) : NULL;
> +
> + break;
> + default:
> + return false;
> + }
> + }
> +
> + va_end(va);
> +
> + return true;
> +}
> diff --git a/src/json.h b/src/json.h
> new file mode 100644
> index 00000000..202445ee
> --- /dev/null
> +++ b/src/json.h
> @@ -0,0 +1,84 @@
> +/*
> + *
> + * Wireless daemon for Linux
> + *
> + * Copyright (C) 2021 Intel Corporation. All rights reserved.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +struct json_iter;
> +
> +/*
> + * Identical to JSMN types
> + */
> +enum json_type {
> + JSON_UNDEFINED = 0,
> + JSON_OBJECT = 1 << 0,
> + JSON_ARRAY = 1 << 1,
> + JSON_STRING = 1 << 2,
> + JSON_PRIMITIVE = 1 << 3
> +};
> +
> +enum json_flag {
> + JSON_FLAG_INVALID = 0,
> + JSON_FLAG_MANDATORY = 1,
> + JSON_FLAG_OPTIONAL = 2,
> +};
> +
> +#define JSON_MANDATORY(key, type, out) \
> + JSON_FLAG_MANDATORY, (key), (type), (out)
> +
> +#define JSON_OPTIONAL(key, type, out) \
> + JSON_FLAG_OPTIONAL, (key), (type), (out)
> +
> +#define JSON_STR_EQ(iov, s) \
> + ((iov)->iov_len == strlen((s)) && \
> + !memcmp((iov)->iov_base, (s), (iov)->iov_len))
> +
> +#define JSON_TO_STR(iov) \
> +({ \
> + char *_tmp = l_malloc((iov)->iov_len + 1); \
> + memcpy(_tmp, (iov)->iov_base, (iov)->iov_len); \
> + _tmp[(iov)->iov_len] = '\0'; \
> + _tmp; \
> +})
> +
> +struct json_iter *json_iter_new(const char *json, size_t json_len);
> +void json_iter_free(struct json_iter *iter);
> +
> +/*
> + * Parse an arbitrary number of key/value pairs from a JSON iterator. Initially
> + * a new JSON iterator should be created with json_iter_new() and, when done,
> + * freed with json_iter_free. Nested objects are also parsed with this function
> + * but these iterators should not be freed and are tracked by the original
> + * iterator.
> + *
> + * Arguments should be specified using JSON_MANDATORY or JSON_OPTIONAL:
> + *
> + * r = json_iter_parse(iter, JSON_MANDATORY("mykey", JSON_STRING, &strvalue),
> + * JSON_OPTIONAL("optkey", JSON_STRING, &optvalue));
> + *
> + * String values should be of type struct iovec *
> + * Object values should be of type struct json_iter **
> + *
> + * No other types are supported at this time, and json_iter_parse will fail if
> + * other types are encountered.
> + *
> + * JSON_OPTIONAL string values will have a NULL/0 iov_base/iov_len if they were
> + * not found. JSON_OPTIONAL object value iterators will be NULL if not found.
> + */
> +bool json_iter_parse(struct json_iter *iter, ...);
Just a nit, but it is a bit clearer to start the '...' after either a
printf-style string or an argument that could carry the sentintel value. For
example:
int nl80211_parse_attrs(struct l_genl_msg *msg, int tag, ...);
It isn't much, but gives just a bit extra context for how the API is supposed to
be used. So in the case of json_iter_parse it would be enum json_flag.
Though really, I wonder why json_iter_parse doesn't use the sequence of:
(json_type, key, value, flags) ?
Regards,
-Denis
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 2/3] json: introduce JSON module
@ 2021-12-08 21:36 James Prestwood
0 siblings, 0 replies; 4+ messages in thread
From: James Prestwood @ 2021-12-08 21:36 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 10153 bytes --]
This is a minimal wrapper around jsmn.h to make things a bit easier
for iterating through a JSON object.
An iterator can be created/destroyed with json_iter_{new,free} and
values can be parsed from this iterator using json_iter_parse.
Arguments to json_iter_parse should use JSON_MANDATORY/JSON_OPTIONAL
macros, and terminate the argument list with JSON_FLAG_INVALID.
The value pointer in the MANDATORY/OPTIONAL macros can be NULL, in
which case the presence of the key/value is checked but no data is
returned out.
Currently only JSON_STRING and JSON_OBJECT types are supported. String
values should be of type struct iovec *, and objects should be of
type struct json_iter **. These nested objects can be parsed again
using json_iter_parse and should not be freed (the original iterator
keeps track of nested object iterators).
---
Makefile.am | 1 +
src/json.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/json.h | 84 ++++++++++++++++++++
3 files changed, 307 insertions(+)
create mode 100644 src/json.c
create mode 100644 src/json.h
v2:
* Re-wrote to remove callback style parsing. Instead the expected values
are provided to the parser directly.
diff --git a/Makefile.am b/Makefile.am
index 7fe4b5c3..1ae7e3c0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -249,6 +249,7 @@ src_iwd_SOURCES = src/main.c linux/nl80211.h src/iwd.h src/missing.h \
src/sysfs.h src/sysfs.c \
src/offchannel.h src/offchannel.c \
src/dpp-util.h src/dpp-util.c \
+ src/json.h src/json.c \
$(eap_sources) \
$(builtin_sources)
diff --git a/src/json.c b/src/json.c
new file mode 100644
index 00000000..9f6c744f
--- /dev/null
+++ b/src/json.c
@@ -0,0 +1,222 @@
+/*
+ *
+ * Wireless daemon for Linux
+ *
+ * Copyright (C) 2013-2019 Intel Corporation. All rights reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+
+#include <ell/ell.h>
+
+#include "src/json.h"
+
+#include "shared/jsmn.h"
+
+/* Max number of tokens supported. Increase if larger objects are expected */
+#define JSON_DEFAULT_TOKENS 30
+
+#define TOK_LEN(token) (token)->end - (token)->start
+#define TOK_PTR(json, token) (void *)((json) + (token)->start)
+
+struct json_iter {
+ const char *json;
+ size_t json_len;
+ jsmntok_t *tokens;
+ int tokens_len;
+ jsmn_parser *p;
+ struct l_queue *children;
+ bool root : 1;
+};
+
+static struct json_iter *iter_recurse(struct json_iter *parent,
+ jsmntok_t *object)
+{
+ struct json_iter *iter = l_new(struct json_iter, 1);
+
+ iter->json = parent->json;
+ iter->json_len = parent->json_len;
+ iter->tokens = object;
+ iter->children = l_queue_new();
+
+ /*
+ * Since object->size only indicates the number of keys in an object
+ * we cannot easily calculate the number of tokens remaining (for this
+ * object) since each object may have nested objects. Set tokens_len to
+ * the number of tokens remaining in the overall list after 'object'
+ */
+ iter->tokens_len = parent->tokens_len -
+ ((object - parent->tokens) % sizeof(jsmntok_t));
+
+ l_queue_push_head(parent->children, iter);
+
+ return iter;
+}
+
+struct json_iter *json_iter_new(const char *json, size_t json_len)
+{
+ struct json_iter *iter = l_new(struct json_iter, 1);
+
+ iter->json = json;
+ iter->json_len = json_len;
+ iter->p = l_new(jsmn_parser, 1);
+ iter->tokens = l_new(jsmntok_t, JSON_DEFAULT_TOKENS);
+ iter->root = true;
+
+ jsmn_init(iter->p);
+ iter->tokens_len = jsmn_parse(iter->p, iter->json, iter->json_len,
+ iter->tokens, JSON_DEFAULT_TOKENS);
+ if (iter->tokens_len < 0) {
+ json_iter_free(iter);
+ return NULL;
+ }
+
+ iter->children = l_queue_new();
+
+ return iter;
+}
+
+static void json_iter_destroy(void *data)
+{
+ struct json_iter *iter = data;
+
+ json_iter_free(iter);
+}
+
+void json_iter_free(struct json_iter *iter)
+{
+ if (iter->root) {
+ l_free(iter->p);
+ l_free(iter->tokens);
+ }
+
+ l_queue_destroy(iter->children, json_iter_destroy);
+ l_free(iter);
+}
+
+static jsmntok_t *next_key(struct json_iter *iter, jsmntok_t *current)
+{
+ jsmntok_t *value;
+ int i;
+
+ /*
+ * In theory JSMN should prevent any of these bounds checks from failing
+ */
+ if (current + 1 > iter->tokens + iter->tokens_len)
+ return NULL;
+
+ value = current + 1;
+
+ if (value->size == 0)
+ return value + 1;
+
+ for (i = 0; i < value->size; i++)
+ value = next_key(iter, value);
+
+ return value;
+}
+
+/*
+ * Checks that the keys value (t + 1) is in bounds, the value type matches,
+ * key length matches, and key string match
+ */
+#define TOKEN_MATCHES(iter, t, key, type) \
+ (t) + 1 < iter->tokens + iter->tokens_len && \
+ (jsmntype_t)(type) == ((t) + 1)->type && \
+ TOK_LEN((t)) == (int)strlen((key)) && \
+ !memcmp(TOK_PTR((iter)->json, (t)), (key), TOK_LEN((t)))
+
+bool json_iter_parse(struct json_iter *iter, ...)
+{
+ va_list va;
+ int i;
+ int num = iter->tokens->size;
+ jsmntok_t *next;
+ struct iovec *sval;
+ struct json_iter **oval;
+
+ /* Empty object {} */
+ if (iter->tokens_len < 1)
+ return false;
+
+ va_start(va, iter);
+
+ while (true) {
+ enum json_flag flag;
+ char *key;
+ enum json_type type;
+ void *value;
+ jsmntok_t *v = NULL;
+
+ flag = va_arg(va, enum json_flag);
+
+ if (flag == JSON_FLAG_INVALID)
+ break;
+
+ key = va_arg(va, char *);
+ type = va_arg(va, enum json_type);
+ value = va_arg(va, void *);
+
+ next = iter->tokens + 1;
+
+ /* Iterate over this objects keys */
+ for (i = 0; i < num; i++) {
+ if (TOKEN_MATCHES(iter, next, key, type)) {
+ v = next + 1;
+ break;
+ }
+
+ next = next_key(iter, next);
+ if (!next)
+ return false;
+ }
+
+ if (flag == JSON_FLAG_MANDATORY && !v)
+ return false;
+
+ switch (type) {
+ case JSON_STRING:
+ if (!value)
+ continue;
+
+ sval = value;
+ sval->iov_base = v ? TOK_PTR(iter->json, v) : NULL;
+ sval->iov_len = v ? TOK_LEN(v) : 0;
+
+ break;
+ case JSON_OBJECT:
+ if (!value)
+ continue;
+
+ oval = value;
+ *oval = v ? iter_recurse(iter, v) : NULL;
+
+ break;
+ default:
+ return false;
+ }
+ }
+
+ va_end(va);
+
+ return true;
+}
diff --git a/src/json.h b/src/json.h
new file mode 100644
index 00000000..202445ee
--- /dev/null
+++ b/src/json.h
@@ -0,0 +1,84 @@
+/*
+ *
+ * Wireless daemon for Linux
+ *
+ * Copyright (C) 2021 Intel Corporation. All rights reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+struct json_iter;
+
+/*
+ * Identical to JSMN types
+ */
+enum json_type {
+ JSON_UNDEFINED = 0,
+ JSON_OBJECT = 1 << 0,
+ JSON_ARRAY = 1 << 1,
+ JSON_STRING = 1 << 2,
+ JSON_PRIMITIVE = 1 << 3
+};
+
+enum json_flag {
+ JSON_FLAG_INVALID = 0,
+ JSON_FLAG_MANDATORY = 1,
+ JSON_FLAG_OPTIONAL = 2,
+};
+
+#define JSON_MANDATORY(key, type, out) \
+ JSON_FLAG_MANDATORY, (key), (type), (out)
+
+#define JSON_OPTIONAL(key, type, out) \
+ JSON_FLAG_OPTIONAL, (key), (type), (out)
+
+#define JSON_STR_EQ(iov, s) \
+ ((iov)->iov_len == strlen((s)) && \
+ !memcmp((iov)->iov_base, (s), (iov)->iov_len))
+
+#define JSON_TO_STR(iov) \
+({ \
+ char *_tmp = l_malloc((iov)->iov_len + 1); \
+ memcpy(_tmp, (iov)->iov_base, (iov)->iov_len); \
+ _tmp[(iov)->iov_len] = '\0'; \
+ _tmp; \
+})
+
+struct json_iter *json_iter_new(const char *json, size_t json_len);
+void json_iter_free(struct json_iter *iter);
+
+/*
+ * Parse an arbitrary number of key/value pairs from a JSON iterator. Initially
+ * a new JSON iterator should be created with json_iter_new() and, when done,
+ * freed with json_iter_free. Nested objects are also parsed with this function
+ * but these iterators should not be freed and are tracked by the original
+ * iterator.
+ *
+ * Arguments should be specified using JSON_MANDATORY or JSON_OPTIONAL:
+ *
+ * r = json_iter_parse(iter, JSON_MANDATORY("mykey", JSON_STRING, &strvalue),
+ * JSON_OPTIONAL("optkey", JSON_STRING, &optvalue));
+ *
+ * String values should be of type struct iovec *
+ * Object values should be of type struct json_iter **
+ *
+ * No other types are supported at this time, and json_iter_parse will fail if
+ * other types are encountered.
+ *
+ * JSON_OPTIONAL string values will have a NULL/0 iov_base/iov_len if they were
+ * not found. JSON_OPTIONAL object value iterators will be NULL if not found.
+ */
+bool json_iter_parse(struct json_iter *iter, ...);
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-12-09 19:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 19:23 [PATCH v2 2/3] json: introduce JSON module Denis Kenzior
-- strict thread matches above, loose matches on Subject: below --
2021-12-09 19:10 James Prestwood
2021-12-09 17:52 Denis Kenzior
2021-12-08 21:36 James Prestwood
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.