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".