linux-ext4 - Re: [PATCH v3 2/5] dumpe2fs: add JSON output of block groups

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DA8C4E1E-D0B1-4B96-9E28-06C16DF7D74D@dilger.ca>
Date:   Thu, 5 Apr 2018 12:40:49 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Viktor Prutyanov <viktor.prutyanov@...tuozzo.com>
Cc:     linux-ext4@...r.kernel.org, Theodore Ts'o <tytso@....edu>,
        Dmitry Monakhov <dmonakhov@...nvz.org>
Subject: Re: [PATCH v3 2/5] dumpe2fs: add JSON output of block groups

On Feb 20, 2018, at 2:59 AM, Viktor Prutyanov <viktor.prutyanov@...tuozzo.com> wrote:
> 
> This patch adds '-j' option for JSON output of block groups information
> 
> Signed-off-by: Viktor Prutyanov <viktor.prutyanov@...tuozzo.com>
> ---
> misc/dumpe2fs.8.in |   3 +
> misc/dumpe2fs.c    | 272 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 270 insertions(+), 5 deletions(-)
> 
> diff --git a/misc/dumpe2fs.8.in b/misc/dumpe2fs.8.in
> index da78d4fc..d03ee8be 100644
> --- a/misc/dumpe2fs.8.in
> +++ b/misc/dumpe2fs.8.in
> @@ -72,6 +72,9 @@ as the pathname to the image file.
> .B \-x
> print the detailed group information block numbers in hexadecimal format
> .TP
> +.B \-j
> +use JSON ouput format
> +.TP
> .B \-V
> print the version number of
> .B dumpe2fs
> diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
> index ca9953a1..319c296b 100644
> --- a/misc/dumpe2fs.c
> +++ b/misc/dumpe2fs.c
> @@ -42,6 +42,7 @@ extern int optind;
> 
> #include "support/nls-enable.h"
> #include "support/plausible.h"
> +#include "support/json-out.h"
> #include "../version.h"
> 
> #define in_use(m, x)	(ext2fs_test_bit ((x), (m)))
> @@ -53,7 +54,7 @@ static int blocks64 = 0;
> 
> static void usage(void)
> {
> -	fprintf(stderr, _("Usage: %s [-bfghixV] [-o superblock=<num>] "
> +	fprintf(stderr, _("Usage: %s [-bfghixjV] [-o superblock=<num>] "
> 		 "[-o blocksize=<num>] device\n"), program_name);
> 	exit(1);
> }
> @@ -69,6 +70,17 @@ static void print_number(unsigned long long num)
> 		printf("%llu", num);
> }
> 
> +static void snprint_number(char *str, size_t size, unsigned long long num)
> +{
> +	if (hex_format) {
> +		if (blocks64)
> +			snprintf(str, size, "0x%08llx", num);
> +		else
> +			snprintf(str, size, "0x%04llx", num);
> +	} else
> +		snprintf(str, size, "%llu", num);
> +}

Instead of adding another helper to print the formatted value into a
temporary string buffer, what might be more useful is to have a helper
that is just generating the format string and use that in the several
places that need it.  Then, instead of using snprint_number() you can
use json_add_fmt_str() and use the helper to generate the fmt string.

static inline const char *number_fmt(void)
{
	return hex_format ? (blocks64 ? "%#08llx" : "%#04llx") : "%llu";
}

static print_number(unsigned long long num)
{
	printf(number_fmt(), num);
}

> +static struct json_obj *json_create_range_obj(unsigned long long a,
> +		       unsigned long long b)
> +{
> +	struct json_obj *obj = json_obj_create();
> +	char buf[32];
> +	const char *fmt = hex_format ? (blocks64 ? "0x%08llx" : "0x%04llx") : "%llu";
> +
> +	json_obj_add_fmt_buf_str(obj, "start", buf, sizeof(buf), fmt, a);
> +	json_obj_add_fmt_buf_str(obj, "len", buf, sizeof(buf), fmt, b - a + 1);

This can then use number_fmt() instead of duplicating that logic to set "fmt".

> @@ -321,6 +413,165 @@ static void list_desc(ext2_filsys fs, int grp_only)
> 
> +static void fill_json_desc(struct json_obj *obj, ext2_filsys fs)
> +{

This function is duplicating a LOT of logic.  It would be better if this
is restructured to use common logic and only have the output format be
different (either JSON or existing format).

> +	unsigned long i;
> +	blk64_t	first_block, last_block;
> +	blk64_t	super_blk, old_desc_blk, new_desc_blk;
> +	char *block_bitmap=NULL, *inode_bitmap=NULL;
> +	const char *units = "blocks";
> +	int inode_blocks_per_group, old_desc_blocks, reserved_gdt;
> +	int		block_nbytes, inode_nbytes;
> +	int has_super;
> +	blk64_t		blk_itr = EXT2FS_B2C(fs, fs->super->s_first_data_block);
> +	ext2_ino_t	ino_itr = 1;
> +	errcode_t	retval;
> +	struct json_list *desc_list = json_list_create_in_obj(obj, "desc", JSON_VAL_OBJECT);

(style) wrap line at 80 chars, everywhere in this function

Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)