On Wed, Nov 13, 2019 at 02:22:34PM +0100, Patrick Steinhardt wrote:
> While the newly added jsmn library provides the parsing interface, it
> does not provide any kind of interface to act on parsed tokens. Instead,
> the caller is expected to handle pointer arithmetics inside of the token
> array in order to extract required information. While simple, this
> requires users to know some of the inner workings of the library and is
> thus quite an unintuitive interface.
>
> This commit adds a new interface on top of the jsmn parser that provides
> convenience functions to retrieve values from the parsed json type,
> `grub_json_t`.
>
> Signed-off-by: Patrick Steinhardt <p...@pks.im>
> ---
> grub-core/lib/json/json.c | 212 ++++++++++++++++++++++++++++++++++++++
> grub-core/lib/json/json.h | 92 +++++++++++++++++
> 2 files changed, 304 insertions(+)
> create mode 100644 grub-core/lib/json/json.h
>
> diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
> index 2bddd8c46..ec971305a 100644
> --- a/grub-core/lib/json/json.c
> +++ b/grub-core/lib/json/json.c
> @@ -17,7 +17,219 @@
> */
>
> #include <grub/dl.h>
> +#include <grub/mm.h>
>
> +#define JSMN_STATIC
> #include "jsmn.h"
> +#include "json.h"
>
> GRUB_MOD_LICENSE ("GPLv3");
> +
> +grub_err_t
> +grub_json_parse (grub_json_t **out, char *string, grub_size_t string_len)
> +{
> + grub_json_t *json = NULL;
> + jsmn_parser parser;
> + grub_err_t err;
> + int jsmn_err;
> +
> + json = grub_zalloc (sizeof (*json));
> + if (!json)
> + return GRUB_ERR_OUT_OF_MEMORY;
> + json->idx = 0;
This initialization is redundant.
> + json->string = string;
> + if (!json->string)
> + {
> + err = GRUB_ERR_OUT_OF_MEMORY;
Hmmm??? GRUB_ERR_BAD_ARGUMENT??? And please check string instead of
json->string. Additionally, if you check it just at the beginning of
the function you can do "return GRUB_ERR_BAD_ARGUMENT" immediately.
> + goto out;
> + }
> +
> + jsmn_init(&parser);
> + jsmn_err = jsmn_parse (&parser, string, string_len, NULL, 0);
> + if (jsmn_err <= 0)
> + {
> + err = GRUB_ERR_BAD_ARGUMENT;
> + goto out;
> + }
> +
> + json->tokens = grub_malloc (sizeof (jsmntok_t) * jsmn_err);
> + if (!json->tokens)
> + {
> + err = GRUB_ERR_OUT_OF_MEMORY;
> + goto out;
> + }
> +
> + jsmn_init(&parser);
Do you need to run jsmn_init() twice? By the way, missing space before "(".
> + jsmn_err = jsmn_parse (&parser, string, string_len, json->tokens,
> jsmn_err);
Could you shortly explain before first jsmn_parse() call what are you
doing there? And maybe add a comment before this jsmn_parse() call too.
> + if (jsmn_err <= 0)
> + {
> + err = GRUB_ERR_BAD_ARGUMENT;
> + goto out;
> + }
> +
> + err = GRUB_ERR_NONE;
Please initialize err in the variables definition section.
> + *out = json;
Please add an empty line here.
> +out:
Oh, out label name clashes with out argument name. Please rename out label
to err and err variable to ret. And please add a space before label name.
> + if (err && json)
> + {
> + grub_free (json->string);
> + grub_free (json->tokens);
> + grub_free (json);
> + }
> + return err;
> +}
> +
> +void
> +grub_json_free (grub_json_t *json)
> +{
> + if (json)
> + {
> + grub_free (json->tokens);
> + grub_free (json);
> + }
> +}
> +
> +grub_size_t
> +grub_json_getsize (const grub_json_t *json)
> +{
> + jsmntok_t *p = &((jsmntok_t *)json->tokens)[json->idx];
Please add an empty line here.
> + return p->size;
> +}
> +
> +grub_json_type_t
> +grub_json_gettype (const grub_json_t *json)
> +{
> + jsmntok_t *p = &((jsmntok_t *)json->tokens)[json->idx];
Ditto...
> + switch (p->type)
> + {
> + case JSMN_OBJECT:
> + return GRUB_JSON_OBJECT;
> + case JSMN_ARRAY:
> + return GRUB_JSON_ARRAY;
> + case JSMN_STRING:
> + return GRUB_JSON_STRING;
> + case JSMN_PRIMITIVE:
> + return GRUB_JSON_PRIMITIVE;
> + default:
> + break;
You do not need break here.
> + }
> + return GRUB_JSON_UNDEFINED;
> +}
> +
> +grub_err_t
> +grub_json_getchild(grub_json_t *out, const grub_json_t *parent, grub_size_t
> n)
> +{
> + jsmntok_t *p = &((jsmntok_t *)parent->tokens)[parent->idx];
Should not you check parent for NULL first? Same thing for functions
above and below.
> + grub_size_t offset = 1;
> +
> + if (n >= (unsigned) p->size)
Should not you cast to grub_size_t? Or should n be type of p->size?
Then you would avoid a cast.
> + return GRUB_ERR_BAD_ARGUMENT;
> +
> + while (n--)
> + n += p[offset++].size;
> +
> + out->string = parent->string;
> + out->tokens = parent->tokens;
> + out->idx = parent->idx + offset;
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +grub_err_t
> +grub_json_getvalue(grub_json_t *out, const grub_json_t *parent, const char
> *key)
> +{
> + grub_size_t i;
> +
> + if (grub_json_gettype (parent) != GRUB_JSON_OBJECT)
> + return GRUB_ERR_BAD_ARGUMENT;
> +
> + for (i = 0; i < grub_json_getsize (parent); i++)
> + {
> + grub_json_t child;
> + const char *s;
> +
> + if (grub_json_getchild (&child, parent, i) ||
> + grub_json_getstring (&s, &child, NULL) ||
> + grub_strcmp (s, key) != 0)
> + continue;
> +
> + return grub_json_getchild (out, &child, 0);
> + }
> +
> + return GRUB_ERR_FILE_NOT_FOUND;
> +}
> +
> +static grub_err_t
> +get_value (grub_json_type_t *out_type, const char **out_string, const
> grub_json_t *parent, const char *key)
> +{
> + const grub_json_t *p = parent;
> + grub_json_t child;
> + jsmntok_t *tok;
> +
> + if (key)
> + {
> + grub_err_t err = grub_json_getvalue (&child, parent, key);
Please define ret instead of err at the beginning of the function and use
it here.
> + if (err)
> + return err;
> + p = &child;
> + }
> +
> + tok = &((jsmntok_t *) p->tokens)[p->idx];
> + p->string[tok->end] = '\0';
> +
> + *out_string = p->string + tok->start;
> + *out_type = grub_json_gettype (p);
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +grub_err_t
> +grub_json_getstring (const char **out, const grub_json_t *parent, const char
> *key)
> +{
> + grub_json_type_t type;
> + const char *value;
> + grub_err_t err;
> +
> + err = get_value(&type, &value, parent, key);
Please use ret name instead of err everywhere.
> + if (err)
> + return err;
> + if (type != GRUB_JSON_STRING)
> + return GRUB_ERR_BAD_ARGUMENT;
> +
> + *out = value;
> + return GRUB_ERR_NONE;
> +}
> +
> +grub_err_t
> +grub_json_getuint64(grub_uint64_t *out, const grub_json_t *parent, const
> char *key)
> +{
> + grub_json_type_t type;
> + const char *value;
> + grub_err_t err;
Ditto...
> + err = get_value(&type, &value, parent, key);
> + if (err)
> + return err;
> + if (type != GRUB_JSON_STRING && type != GRUB_JSON_PRIMITIVE)
> + return GRUB_ERR_BAD_ARGUMENT;
> +
> + *out = grub_strtoul (value, NULL, 10);
> + return GRUB_ERR_NONE;
> +}
> +
> +grub_err_t
> +grub_json_getint64(grub_int64_t *out, const grub_json_t *parent, const char
> *key)
> +{
> + grub_json_type_t type;
> + const char *value;
> + grub_err_t err;
> +
> + err = get_value(&type, &value, parent, key);
> + if (err)
> + return err;
> + if (type != GRUB_JSON_STRING && type != GRUB_JSON_PRIMITIVE)
> + return GRUB_ERR_BAD_ARGUMENT;
> +
> + *out = grub_strtol (value, NULL, 10);
> + return GRUB_ERR_NONE;
> +}
> diff --git a/grub-core/lib/json/json.h b/grub-core/lib/json/json.h
> new file mode 100644
> index 000000000..db8160c3a
> --- /dev/null
> +++ b/grub-core/lib/json/json.h
> @@ -0,0 +1,92 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2019 Free Software Foundation, Inc.
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef GRUB_JSON_JSON_H
> +#define GRUB_JSON_JSON_H 1
> +
> +#include <grub/types.h>
> +
> +enum grub_json_type
> +{
> + /* Unordered collection of key-value pairs. */
> + GRUB_JSON_OBJECT,
> + /* Ordered list of zero or more values. */
> + GRUB_JSON_ARRAY,
> + /* Zero or more Unicode characters. */
> + GRUB_JSON_STRING,
> + /* Number, boolean or empty value. */
> + GRUB_JSON_PRIMITIVE,
> + /* Invalid token. */
> + GRUB_JSON_UNDEFINED,
> +};
> +typedef enum grub_json_type grub_json_type_t;
> +
> +struct grub_json
> +{
> + void *tokens;
> + char *string;
> + grub_size_t idx;
> +};
> +typedef struct grub_json grub_json_t;
> +
> +/* Parse a JSON-encoded string. Note that the string passed to
> + * this function will get modified on subsequent calls to
> + * `grub_json_get*`. Returns the root object of the parsed JSON
> + * object, which needs to be free'd via `grub_json_free`.
Please use grub_json_free() instead of `grub_json_free` in every
comment. Same for grub_json_get*(), etc.
And more about comments in the GRUB code you can find here:
https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Multi_002dLine-Comments
> + */
> +grub_err_t
> +grub_json_parse (grub_json_t **out, char *string, grub_size_t string_len);
extern grub_err_t EXPORT_FUNC(grub_json_parse) (grub_json_t **out,
char *string,
grub_size_t string_len);
> +/* Free the structure and its contents. The string passed to
> + * `grub_json_parse` will not be free'd.
> + */
> +void
> +grub_json_free (grub_json_t *json);
extern void EXPORT_FUNC(grub_json_free) (grub_json_t *json);
...and below...
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
