On Feb 20, 2018, at 2:59 AM, Viktor Prutyanov <viktor.prutyanov@xxxxxxxxxxxxx> wrote:
>
> This patch adds JSON objects and lists and methods to print them
This patch also adds the JSON routines to dumpe2fs, but in a way that is
non-functional. Those changes to dumpe2fs should be moved into the 2/3
patch so that git bisect does not break if only this patch is applied.
Secondly, my preference would be if the JSON functionality depended on a
"configure --enable-json" check that set JSON_CMT, JSON_MAN, and ENABLE_JSON
appropriately, so that this functionality could be #ifdef'd out if not needed.
It is fine to have JSON formatting enabled by default, but I don't think it
is needed in all environments.
More comments inline.
> Signed-off-by: Viktor Prutyanov <viktor.prutyanov@xxxxxxxxxxxxx>
> ---
>
> new file mode 100644
> index 00000000..8c822f35
> --- /dev/null
> +++ b/lib/support/json-out.c
> @@ -0,0 +1,326 @@
> +/*
> + * json-out.c -- JSON output
> + *
> + * Copyright (c) 2018 Virtuozzo International GmbH
> + *
> + * %Begin-Header%
> + * This file may be redistributed under the terms of the GNU Public
> + * License.
> + * %End-Header%
>
> +static struct json_pair *json_obj_add_pair(struct json_obj *obj, const char *key, enum json_val_type type)
> +{
> + struct json_pair *new_pair = xmalloc(sizeof(struct json_pair));
(style) blank line after variable declaration.
> + new_pair->key = xstrdup(key);
> + new_pair->next = NULL;
> + new_pair->type = type;
> +
> + if (obj->pair) {
> + struct json_pair *pair;
> +
> + for (pair = obj->pair; pair && pair->next; pair = pair->next);
> + pair->next = new_pair;
> + } else
> + obj->pair = new_pair;
(style) use {} in "else" block when "if" block has {}
> +void json_obj_add_str(struct json_obj *obj, const char *key, const char *str)
> +{
> + struct json_pair *new_pair = json_obj_add_pair(obj, key, JSON_VAL_STRING);
> +
> + new_pair->val.str = xstrdup(str);
> +}
> +
> +void json_obj_add_fmt_buf_str(struct json_obj *obj, const char *key, char *buf, size_t size, const char *fmt, ...)
(style) wrap line at 80 chars
> +{
> + va_list ap;
> +
> + va_start(ap, fmt);
> + vsnprintf(buf, size, fmt, ap);
> + va_end(ap);
> +
> + json_obj_add_str(obj, key, buf);
> +}
> +
> +void json_obj_add_fmt_str(struct json_obj *obj, const char *key, size_t size, const char *fmt, ...)
(style) wrap line at 80 chars
> +{
> + va_list ap;
> + char *buf = malloc(size);
> +
> + va_start(ap, fmt);
> + vsnprintf(buf, size, fmt, ap);
> + va_end(ap);
> +
> + json_obj_add_str(obj, key, buf);
> + free(buf);
> +}
> +
> +void json_obj_add_list(struct json_obj *obj, const char *key, struct json_list *list)
...
> +{
> + struct json_pair *new_pair = json_obj_add_pair(obj, key, JSON_VAL_LIST);
> +
> + new_pair->val.list = list;
> +}
> +
> +void json_obj_add_obj(struct json_obj *obj, const char *key, struct json_obj *new_obj)
...
> +{
> + struct json_pair *new_pair = json_obj_add_pair(obj, key, JSON_VAL_OBJECT);
> +
> + new_pair->val.obj = new_obj;
> +}
> +
> +static void json_pair_print_json(struct json_pair *pair, int ind_lvl);
> +static void json_list_node_print_json(struct json_list_node *node, enum json_val_type type, int ind_lvl);
...
> +
> +static void
> +print_ind(int ind_lvl)
(style) "static void" should be on the same line as "print_ind()".
IMHO, calling this function "print_indent()" would be more clear. I thought
it was "print_index()"...
> +{
> + int i;
> +
> + putchar('\n');
> + for (i = 0; i < ind_lvl; i++)
> + fputs(" ", stdout);
(style) this line should be indented
> +static void
> +json_list_node_print_json(struct json_list_node *node, enum json_val_type type, int ind_lvl)
(style) "static void" on same line as function, wrap at 80 chars
> diff --git a/lib/support/json-out.h b/lib/support/json-out.h
> new file mode 100644
> index 00000000..0132bd2c
> --- /dev/null
> +++ b/lib/support/json-out.h
> @@ -0,0 +1,69 @@
>
> +void json_obj_add_list(struct json_obj *obj, const char *key, struct json_list *list);
> +void json_obj_add_obj(struct json_obj *obj, const char *key, struct json_obj *new_obj);
> +void json_obj_add_fmt_str(struct json_obj *obj, const char *key, size_t size, const char *fmt, ...);
> +void json_obj_add_fmt_buf_str(struct json_obj *obj, const char *key, char *buf, size_t size, const char *fmt, ...);
> +struct json_list *json_list_create_in_obj(struct json_obj *parent_obj, char *key, enum json_val_type type);
(style) wrap lines at 80 chars
> diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
> index 395ea9ee..ca9953a1 100644
> --- a/misc/dumpe2fs.c
> +++ b/misc/dumpe2fs.c
> @@ -607,7 +607,12 @@ try_open_again:
> goto just_descriptors;
> list_super (fs->super);
> if (ext2fs_has_feature_journal_dev(fs->super)) {
> - print_journal_information(fs);
> + print_journal_information(fs, dump_obj);
> + if (json) {
> + json_obj_print_json(dump_obj, 0);
> + putchar('\n');
> + json_obj_delete(dump_obj);
> + }
(defect) the "json" variable is not yet declared in this patch, it
is only added in patch 2/3, so all of these changes should also be
moved to the 2/3 patch.
> ext2fs_close_free(&fs);
> exit(0);
> }
> @@ -616,6 +621,11 @@ try_open_again:
> print_inline_journal_information(fs);
> list_bad_blocks(fs, 0);
> if (header_only) {
> + if (json) {
> + json_obj_print_json(dump_obj, 0);
> + putchar('\n');
> + json_obj_delete(dump_obj);
> + }
> ext2fs_close_free(&fs);
> exit (0);
> }
> @@ -636,6 +646,11 @@ just_descriptors:
> error_message(retval));
> }
> }
> + if (json) {
> + json_obj_print_json(dump_obj, 0);
> + putchar('\n');
> + json_obj_delete(dump_obj);
> + }
> ext2fs_close_free(&fs);
> remove_error_table(&et_ext2_error_table);
> exit (0);
> --
> 2.14.1
>
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
