Re: [PATCH iproute2 net-next v3 1/5] json_writer: allow base json data type to be array or object

[Stephen Hemminger]

On Tue, 21 Jun 2016 09:24:50 -0700
Anuradha Karuppiah  wrote:

> On Tue, Jun 21, 2016 at 9:12 AM, Stephen Hemminger
>  wrote:
> > On Mon, 20 Jun 2016 23:39:43 -0700
> > Roopa Prabhu  wrote:
> >
> >> From: Anuradha Karuppiah 
> >>
> >> This patch adds a type qualifier to json_writer. Type can be a
> >> json object or array. This can be extended to other types like
> >> json-string, json-number etc in the future.
> >>
> >> Signed-off-by: Anuradha Karuppiah 
> >
> > Since json writer is not used in many places yet, why not just
> > get rid of the automatic object in the constructor.
> 
> I wanted to force the external api to start with an json-object or
> json-array. It reduces the chance of mistakes vs. a typeless
> constructor. With a typeless constructor you can accidentally end up
> with a json output that doesn't pass json lint; especially if optional
> params are being suppressed at different places.

Still, this is not how jsonwriter works in .NET, Android, or Java.
It is easily confusing to developers if similar API's behave
differently.  Kind of like if printf() always appended a new line
on some platforms.

Re: [PATCH iproute2 net-next v3 1/5] json_writer: allow base json data type to be array or object

[Anuradha Karuppiah]

On Tue, Jun 21, 2016 at 9:12 AM, Stephen Hemminger
 wrote:
> On Mon, 20 Jun 2016 23:39:43 -0700
> Roopa Prabhu  wrote:
>
>> From: Anuradha Karuppiah 
>>
>> This patch adds a type qualifier to json_writer. Type can be a
>> json object or array. This can be extended to other types like
>> json-string, json-number etc in the future.
>>
>> Signed-off-by: Anuradha Karuppiah 
>
> Since json writer is not used in many places yet, why not just
> get rid of the automatic object in the constructor.

I wanted to force the external api to start with an json-object or
json-array. It reduces the chance of mistakes vs. a typeless
constructor. With a typeless constructor you can accidentally end up
with a json output that doesn't pass json lint; especially if optional
params are being suppressed at different places.

>
>
>
> diff --git a/lib/json_writer.c b/lib/json_writer.c
> index 2af16e1..5e588d8 100644
> --- a/lib/json_writer.c
> +++ b/lib/json_writer.c
> @@ -102,7 +102,6 @@ json_writer_t *jsonw_new(FILE *f)
> self->depth = 0;
> self->pretty = false;
> self->sep = '\0';
> -   putc('{', self->out);
> }
> return self;
>  }
> @@ -114,7 +113,6 @@ void jsonw_destroy(json_writer_t **self_p)
>
> assert(self->depth == 0);
> jsonw_eol(self);
> -   fputs("}\n", self->out);
> fflush(self->out);
> free(self);
> *self_p = NULL;
> diff --git a/misc/ifstat.c b/misc/ifstat.c
> index abbb4e7..d9a7e50 100644
> --- a/misc/ifstat.c
> +++ b/misc/ifstat.c
> @@ -246,7 +246,6 @@ static void dump_raw_db(FILE *fp, int to_hist)
> h = hist_db;
> if (jw) {
> jsonw_pretty(jw, pretty);
> -   jsonw_name(jw, info_source);
> jsonw_start_object(jw);
> } else
> fprintf(fp, "#%s\n", info_source);
> @@ -452,7 +451,6 @@ static void dump_kern_db(FILE *fp)
>
> if (jw) {
> jsonw_pretty(jw, pretty);
> -   jsonw_name(jw, info_source);
> jsonw_start_object(jw);
> } else
> print_head(fp);
> @@ -466,8 +464,10 @@ static void dump_kern_db(FILE *fp)
> else
> print_one_if(fp, n, n->val);
> }
> -   if (json_output)
> -   fprintf(fp, "\n} }\n");
> +   if (jw) {
> +   jsonw_end_object(jw);
> +   jsonw_destroy();
> +   }
>  }
>
>  static void dump_incr_db(FILE *fp)
> @@ -478,7 +478,6 @@ static void dump_incr_db(FILE *fp)
> h = hist_db;
> if (jw) {
> jsonw_pretty(jw, pretty);
> -   jsonw_name(jw, info_source);
> jsonw_start_object(jw);
> } else
> print_head(fp);
> diff --git a/misc/nstat.c b/misc/nstat.c
> index a9e0f20..411cd87 100644
> --- a/misc/nstat.c
> +++ b/misc/nstat.c
> @@ -285,7 +285,6 @@ static void dump_kern_db(FILE *fp, int to_hist)
> h = hist_db;
> if (jw) {
> jsonw_pretty(jw, pretty);
> -   jsonw_name(jw, info_source);
> jsonw_start_object(jw);
> } else
> fprintf(fp, "#%s\n", info_source);

Re: [PATCH iproute2 net-next v3 1/5] json_writer: allow base json data type to be array or object

[Stephen Hemminger]

On Mon, 20 Jun 2016 23:39:43 -0700
Roopa Prabhu  wrote:

> From: Anuradha Karuppiah 
> 
> This patch adds a type qualifier to json_writer. Type can be a
> json object or array. This can be extended to other types like
> json-string, json-number etc in the future.
> 
> Signed-off-by: Anuradha Karuppiah 

Since json writer is not used in many places yet, why not just
get rid of the automatic object in the constructor.



diff --git a/lib/json_writer.c b/lib/json_writer.c
index 2af16e1..5e588d8 100644
--- a/lib/json_writer.c
+++ b/lib/json_writer.c
@@ -102,7 +102,6 @@ json_writer_t *jsonw_new(FILE *f)
self->depth = 0;
self->pretty = false;
self->sep = '\0';
-   putc('{', self->out);
}
return self;
 }
@@ -114,7 +113,6 @@ void jsonw_destroy(json_writer_t **self_p)
 
assert(self->depth == 0);
jsonw_eol(self);
-   fputs("}\n", self->out);
fflush(self->out);
free(self);
*self_p = NULL;
diff --git a/misc/ifstat.c b/misc/ifstat.c
index abbb4e7..d9a7e50 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -246,7 +246,6 @@ static void dump_raw_db(FILE *fp, int to_hist)
h = hist_db;
if (jw) {
jsonw_pretty(jw, pretty);
-   jsonw_name(jw, info_source);
jsonw_start_object(jw);
} else
fprintf(fp, "#%s\n", info_source);
@@ -452,7 +451,6 @@ static void dump_kern_db(FILE *fp)
 
if (jw) {
jsonw_pretty(jw, pretty);
-   jsonw_name(jw, info_source);
jsonw_start_object(jw);
} else
print_head(fp);
@@ -466,8 +464,10 @@ static void dump_kern_db(FILE *fp)
else
print_one_if(fp, n, n->val);
}
-   if (json_output)
-   fprintf(fp, "\n} }\n");
+   if (jw) {
+   jsonw_end_object(jw);
+   jsonw_destroy();
+   }
 }
 
 static void dump_incr_db(FILE *fp)
@@ -478,7 +478,6 @@ static void dump_incr_db(FILE *fp)
h = hist_db;
if (jw) {
jsonw_pretty(jw, pretty);
-   jsonw_name(jw, info_source);
jsonw_start_object(jw);
} else
print_head(fp);
diff --git a/misc/nstat.c b/misc/nstat.c
index a9e0f20..411cd87 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -285,7 +285,6 @@ static void dump_kern_db(FILE *fp, int to_hist)
h = hist_db;
if (jw) {
jsonw_pretty(jw, pretty);
-   jsonw_name(jw, info_source);
jsonw_start_object(jw);
} else
fprintf(fp, "#%s\n", info_source);

[PATCH iproute2 net-next v3 1/5] json_writer: allow base json data type to be array or object

[Roopa Prabhu]

From: Anuradha Karuppiah 

This patch adds a type qualifier to json_writer. Type can be a
json object or array. This can be extended to other types like
json-string, json-number etc in the future.

Signed-off-by: Anuradha Karuppiah 
---
 include/json_writer.h |  5 +++--
 lib/json_writer.c | 39 +++
 misc/ifstat.c |  6 +++---
 misc/lnstat.c |  2 +-
 misc/nstat.c  |  4 ++--
 5 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/include/json_writer.h b/include/json_writer.h
index ab9a008..e04a40a 100644
--- a/include/json_writer.h
+++ b/include/json_writer.h
@@ -21,8 +21,9 @@
 /* Opaque class structure */
 typedef struct json_writer json_writer_t;
 
-/* Create a new JSON stream */
-json_writer_t *jsonw_new(FILE *f);
+/* Create a new JSON stream with data type */
+json_writer_t *jsonw_new_object(FILE *f);
+json_writer_t *jsonw_new_array(FILE *f);
 /* End output to JSON stream */
 void jsonw_destroy(json_writer_t **self_p);
 
diff --git a/lib/json_writer.c b/lib/json_writer.c
index 2af16e1..420cd87 100644
--- a/lib/json_writer.c
+++ b/lib/json_writer.c
@@ -22,11 +22,17 @@
 
 #include "json_writer.h"
 
+enum jsonw_data_type {
+   JSONW_TYPE_OBJECT,
+   JSONW_TYPE_ARRAY
+};
+
 struct json_writer {
FILE*out;   /* output file */
unsigneddepth;  /* nesting */
boolpretty; /* optional whitepace */
charsep;/* either nul or comma */
+   int type;   /* currently either object or array */
 };
 
 /* indentation for pretty print */
@@ -94,7 +100,7 @@ static void jsonw_puts(json_writer_t *self, const char *str)
 }
 
 /* Create a new JSON stream */
-json_writer_t *jsonw_new(FILE *f)
+static json_writer_t *jsonw_new(FILE *f, int type)
 {
json_writer_t *self = malloc(sizeof(*self));
if (self) {
@@ -102,11 +108,29 @@ json_writer_t *jsonw_new(FILE *f)
self->depth = 0;
self->pretty = false;
self->sep = '\0';
-   putc('{', self->out);
+   self->type = type;
+   switch (self->type) {
+   case JSONW_TYPE_OBJECT:
+   putc('{', self->out);
+   break;
+   case JSONW_TYPE_ARRAY:
+   putc('[', self->out);
+   break;
+   }
}
return self;
 }
 
+json_writer_t *jsonw_new_object(FILE *f)
+{
+   return jsonw_new(f, JSONW_TYPE_OBJECT);
+}
+
+json_writer_t *jsonw_new_array(FILE *f)
+{
+   return jsonw_new(f, JSONW_TYPE_ARRAY);
+}
+
 /* End output to JSON stream */
 void jsonw_destroy(json_writer_t **self_p)
 {
@@ -114,7 +138,14 @@ void jsonw_destroy(json_writer_t **self_p)
 
assert(self->depth == 0);
jsonw_eol(self);
-   fputs("}\n", self->out);
+   switch (self->type) {
+   case JSONW_TYPE_OBJECT:
+   fputs("}\n", self->out);
+   break;
+   case JSONW_TYPE_ARRAY:
+   fputs("]\n", self->out);
+   break;
+   }
fflush(self->out);
free(self);
*self_p = NULL;
@@ -267,7 +298,7 @@ void jsonw_null_field(json_writer_t *self, const char *prop)
 #ifdef TEST
 int main(int argc, char **argv)
 {
-   json_writer_t *wr = jsonw_new(stdout);
+   json_writer_t *wr = jsonw_new_object(stdout);
 
jsonw_pretty(wr, true);
jsonw_name(wr, "Vyatta");
diff --git a/misc/ifstat.c b/misc/ifstat.c
index abbb4e7..29aa63c 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -240,7 +240,7 @@ static void load_raw_table(FILE *fp)
 
 static void dump_raw_db(FILE *fp, int to_hist)
 {
-   json_writer_t *jw = json_output ? jsonw_new(fp) : NULL;
+   json_writer_t *jw = json_output ? jsonw_new_object(fp) : NULL;
struct ifstat_ent *n, *h;
 
h = hist_db;
@@ -447,7 +447,7 @@ static void print_one_if(FILE *fp, const struct ifstat_ent 
*n,
 
 static void dump_kern_db(FILE *fp)
 {
-   json_writer_t *jw = json_output ? jsonw_new(fp) : NULL;
+   json_writer_t *jw = json_output ? jsonw_new_object(fp) : NULL;
struct ifstat_ent *n;
 
if (jw) {
@@ -473,7 +473,7 @@ static void dump_kern_db(FILE *fp)
 static void dump_incr_db(FILE *fp)
 {
struct ifstat_ent *n, *h;
-   json_writer_t *jw = json_output ? jsonw_new(fp) : NULL;
+   json_writer_t *jw = json_output ? jsonw_new_object(fp) : NULL;
 
h = hist_db;
if (jw) {
diff --git a/misc/lnstat.c b/misc/lnstat.c
index 659a01b..2988e9e 100644
--- a/misc/lnstat.c
+++ b/misc/lnstat.c
@@ -110,7 +110,7 @@ static void print_line(FILE *of, const struct lnstat_file 
*lnstat_files,
 static void print_json(FILE *of, const struct lnstat_file *lnstat_files,
   const struct field_params *fp)
 {
-   json_writer_t *jw = jsonw_new(of);
+   json_writer_t *jw =