Re: [PATCH v2] json_writer: new routines to create data in JSON format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



git@xxxxxxxxxxxxxxxxx writes:

> From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
>
> Add basic routines to generate data in JSON format.

And the point of having capability to write JSON data in our
codebase is...?

> diff --git a/json-writer.c b/json-writer.c
> new file mode 100644
> index 0000000..89a6abb
> --- /dev/null
> +++ b/json-writer.c
> @@ -0,0 +1,321 @@
> +#include "cache.h"
> +#include "json-writer.h"
> +
> +static char g_ch_open[2]  = { '{', '[' };
> +static char g_ch_close[2] = { '}', ']' };

What's "g_" prefix?

> +
> +/*
> + * Append JSON-quoted version of the given string to 'out'.
> + */
> +static void append_quoted_string(struct strbuf *out, const char *in)
> +{
> +	strbuf_addch(out, '"');
> +	for (/**/; *in; in++) {
> +		unsigned char c = (unsigned char)*in;

It is clear enough to lose /**/, i.e.

	for (; *in; in++) {

but for this one. I wonder if

	unsigned char c;
	strbuf_addch(out, '"');
	while ((c = *in++) != '\0') {
 		...

is easier to follow, though.

> +static inline void begin(struct json_writer *jw, int is_array)
> +{
> +	ALLOC_GROW(jw->levels, jw->nr + 1, jw->alloc);
> +
> +	jw->levels[jw->nr].level_is_array = !!is_array;
> +	jw->levels[jw->nr].level_is_empty = 1;

An element of this array is a struct that represents a level, and
everybody who accesses an element of that type knows it is talking
about a level by the field that has the array being named as
.levels[] (also [*1*]).  In such a context, it is a bit too loud to
name the fields with level_$blah.  IOW,

	struct json_writer_level
	{
		unsigned is_array : 1;
		unsigned is_empty : 1;
	};

> +struct json_writer_level
> +{
> +	unsigned level_is_array : 1;
> +	unsigned level_is_empty : 1;
> +};
> +
> +struct json_writer
> +{
> +	struct json_writer_level *levels;
> +	int nr, alloc;
> +	struct strbuf json;
> +};

[Footnote]

*1* I personally prefer to call an array of things "thing[]", not
    "things[]", because then you can refer to an individual element
    e.g. "thing[4]" and read it as "the fourth thing".

    Unless the code often treats an array as a whole, that is, in
    which case, things[] is OK as you'll be calling the whole thing
    with the plural name (e.g. call that function and give all the
    things by passing things[]).

    In this case, one level instance is an element of a stack, and
    the code would be accessing one level at a time most of the
    time, so "writer.level[4].is_empty" would read more naturally
    than "writer.levels[4].level_is_empty".





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux