Re: [LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[John Crispin]

On 17/01/18 10:44, Arjen de Korte wrote:

Citeren John Crispin :


On 07/01/18 18:08, Christian Beier wrote:

The existing read functionality feeds the complete JSON to jshn as a
cmdline argument, leading to `-ash: jshn: Argument list too long`
errors for JSONs bigger than ca. 100KB.

This commit adds the ability to read the JSON directly from a file if
wanted, removing this shell-imposed size limit.

Tested on x86-64 and ar71xx. An mmap()-based solution was also 
evaluated,

but found to make no performance difference on either platform.

Signed-off-by: Christian Beier 


Hi Christian,

comment inline ...


---
 jshn.c | 30 --
 sh/jshn.sh |  4 
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/jshn.c b/jshn.c
index 3188af5..eb72fb7 100644
--- a/jshn.c
+++ b/jshn.c
@@ -25,6 +25,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "list.h"
   #include "avl.h"
@@ -305,7 +307,7 @@ out:
   static int usage(const char *progname)
 {
-    fprintf(stderr, "Usage: %s [-n] [-i] -r |-w\n", 
progname);
+    fprintf(stderr, "Usage: %s [-n] [-i] -r |-R 
|-w\n", progname);

 return 2;
 }
 @@ -338,6 +340,10 @@ int main(int argc, char **argv)
 struct env_var *vars;
 int i;
 int ch;
+    int fd;
+    struct stat sb;
+    char *fbuf;
+    int ret;
   avl_init(_vars, avl_strcmp_var, false, NULL);
 for (i = 0; environ[i]; i++);
@@ -359,7 +365,7 @@ int main(int argc, char **argv)
 avl_insert(_vars, [i].avl);
 }
 -    while ((ch = getopt(argc, argv, "p:nir:w")) != -1) {
+    while ((ch = getopt(argc, argv, "p:nir:R:w")) != -1) {
 switch(ch) {
 case 'p':
 var_prefix = optarg;
@@ -367,6 +373,26 @@ int main(int argc, char **argv)
 break;
 case 'r':
 return jshn_parse(optarg);
+    case 'R':
+    if ((fd = open(optarg, O_RDONLY)) == -1) {
+    fprintf(stderr, "Error opening %s\n", optarg);
+    return 3;
+    }
+    if (fstat(fd, ) == -1) {
+    fprintf(stderr, "Error getting size of %s\n", optarg);
+    close(fd);
+    return 3;
+    }
+    if (!(fbuf = malloc(sb.st_size)) || read(fd, fbuf, 
sb.st_size) != sb.st_size) {

+    fprintf(stderr, "Error reading %s\n", optarg);
+    free(fbuf);


this will blow up if the malloc fails.


How would it? If the malloc fails and returns a NULL pointer, the read 
will not be performed. Free'ing a NULL pointer is allowed, so although 
assigning a value in an if() statement is considered a no-no by some 
(including me), there is no reason for it to blow up.


ok, point taken 




please spli the if clause up into 2 blocks


Agreed (but for a different reason). A memory allocation error is 
different from failure to read from a file and error handlers should 
not treat them the same.



    John


+    close(fd);
+    return 3;
+    }
+    ret = jshn_parse(fbuf);
+    free(fbuf);
+    close(fd);
+    return ret;
 case 'w':
 return jshn_format(no_newline, indent);
 case 'n':
diff --git a/sh/jshn.sh b/sh/jshn.sh
index 1090814..66baccb 100644
--- a/sh/jshn.sh
+++ b/sh/jshn.sh
@@ -180,6 +180,10 @@ json_load() {
 eval "`jshn -r "$1"`"
 }
 +json_load_file() {
+    eval "`jshn -R "$1"`"
+}
+
 json_dump() {
 jshn "$@" ${JSON_PREFIX:+-p "$JSON_PREFIX"} -w
 }



___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev





___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev



___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[Arjen de Korte]

Citeren John Crispin :


On 07/01/18 18:08, Christian Beier wrote:

The existing read functionality feeds the complete JSON to jshn as a
cmdline argument, leading to `-ash: jshn: Argument list too long`
errors for JSONs bigger than ca. 100KB.

This commit adds the ability to read the JSON directly from a file if
wanted, removing this shell-imposed size limit.

Tested on x86-64 and ar71xx. An mmap()-based solution was also evaluated,
but found to make no performance difference on either platform.

Signed-off-by: Christian Beier 


Hi Christian,

comment inline ...


---
 jshn.c | 30 --
 sh/jshn.sh |  4 
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/jshn.c b/jshn.c
index 3188af5..eb72fb7 100644
--- a/jshn.c
+++ b/jshn.c
@@ -25,6 +25,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "list.h"
   #include "avl.h"
@@ -305,7 +307,7 @@ out:
   static int usage(const char *progname)
 {
-   fprintf(stderr, "Usage: %s [-n] [-i] -r |-w\n", progname);
+	fprintf(stderr, "Usage: %s [-n] [-i] -r |-R  
|-w\n", progname);

return 2;
 }
 @@ -338,6 +340,10 @@ int main(int argc, char **argv)
struct env_var *vars;
int i;
int ch;
+   int fd;
+   struct stat sb;
+   char *fbuf;
+   int ret;
avl_init(_vars, avl_strcmp_var, false, NULL);
for (i = 0; environ[i]; i++);
@@ -359,7 +365,7 @@ int main(int argc, char **argv)
avl_insert(_vars, [i].avl);
}
 -  while ((ch = getopt(argc, argv, "p:nir:w")) != -1) {
+   while ((ch = getopt(argc, argv, "p:nir:R:w")) != -1) {
switch(ch) {
case 'p':
var_prefix = optarg;
@@ -367,6 +373,26 @@ int main(int argc, char **argv)
break;
case 'r':
return jshn_parse(optarg);
+   case 'R':
+   if ((fd = open(optarg, O_RDONLY)) == -1) {
+   fprintf(stderr, "Error opening %s\n", optarg);
+   return 3;
+   }
+   if (fstat(fd, ) == -1) {
+   fprintf(stderr, "Error getting size of %s\n", 
optarg);
+   close(fd);
+   return 3;
+   }
+			if (!(fbuf = malloc(sb.st_size)) || read(fd, fbuf, sb.st_size)  
!= sb.st_size) {

+   fprintf(stderr, "Error reading %s\n", optarg);
+   free(fbuf);


this will blow up if the malloc fails.


How would it? If the malloc fails and returns a NULL pointer, the read  
will not be performed. Free'ing a NULL pointer is allowed, so although  
assigning a value in an if() statement is considered a no-no by some  
(including me), there is no reason for it to blow up.



please spli the if clause up into 2 blocks


Agreed (but for a different reason). A memory allocation error is  
different from failure to read from a file and error handlers should  
not treat them the same.



    John


+   close(fd);
+   return 3;
+   }
+   ret = jshn_parse(fbuf);
+   free(fbuf);
+   close(fd);
+   return ret;
case 'w':
return jshn_format(no_newline, indent);
case 'n':
diff --git a/sh/jshn.sh b/sh/jshn.sh
index 1090814..66baccb 100644
--- a/sh/jshn.sh
+++ b/sh/jshn.sh
@@ -180,6 +180,10 @@ json_load() {
eval "`jshn -r "$1"`"
 }
 +json_load_file() {
+   eval "`jshn -R "$1"`"
+}
+
 json_dump() {
jshn "$@" ${JSON_PREFIX:+-p "$JSON_PREFIX"} -w
 }



___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev





___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[John Crispin]

On 07/01/18 18:08, Christian Beier wrote:

The existing read functionality feeds the complete JSON to jshn as a
cmdline argument, leading to `-ash: jshn: Argument list too long`
errors for JSONs bigger than ca. 100KB.

This commit adds the ability to read the JSON directly from a file if
wanted, removing this shell-imposed size limit.

Tested on x86-64 and ar71xx. An mmap()-based solution was also evaluated,
but found to make no performance difference on either platform.

Signed-off-by: Christian Beier 


Hi Christian,

comment inline ...


---
  jshn.c | 30 --
  sh/jshn.sh |  4 
  2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/jshn.c b/jshn.c
index 3188af5..eb72fb7 100644
--- a/jshn.c
+++ b/jshn.c
@@ -25,6 +25,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 
  #include "list.h"
  
  #include "avl.h"

@@ -305,7 +307,7 @@ out:
  
  static int usage(const char *progname)

  {
-   fprintf(stderr, "Usage: %s [-n] [-i] -r |-w\n", progname);
+   fprintf(stderr, "Usage: %s [-n] [-i] -r |-R |-w\n", 
progname);
return 2;
  }
  
@@ -338,6 +340,10 @@ int main(int argc, char **argv)

struct env_var *vars;
int i;
int ch;
+   int fd;
+   struct stat sb;
+   char *fbuf;
+   int ret;
  
  	avl_init(_vars, avl_strcmp_var, false, NULL);

for (i = 0; environ[i]; i++);
@@ -359,7 +365,7 @@ int main(int argc, char **argv)
avl_insert(_vars, [i].avl);
}
  
-	while ((ch = getopt(argc, argv, "p:nir:w")) != -1) {

+   while ((ch = getopt(argc, argv, "p:nir:R:w")) != -1) {
switch(ch) {
case 'p':
var_prefix = optarg;
@@ -367,6 +373,26 @@ int main(int argc, char **argv)
break;
case 'r':
return jshn_parse(optarg);
+   case 'R':
+   if ((fd = open(optarg, O_RDONLY)) == -1) {
+   fprintf(stderr, "Error opening %s\n", optarg);
+   return 3;
+   }
+   if (fstat(fd, ) == -1) {
+   fprintf(stderr, "Error getting size of %s\n", 
optarg);
+   close(fd);
+   return 3;
+   }
+   if (!(fbuf = malloc(sb.st_size)) || read(fd, fbuf, 
sb.st_size) != sb.st_size) {
+   fprintf(stderr, "Error reading %s\n", optarg);
+   free(fbuf);


this will blow up if the malloc fails. please spli the if clause up into 
2 blocks


    John


+   close(fd);
+   return 3;
+   }
+   ret = jshn_parse(fbuf);
+   free(fbuf);
+   close(fd);
+   return ret;
case 'w':
return jshn_format(no_newline, indent);
case 'n':
diff --git a/sh/jshn.sh b/sh/jshn.sh
index 1090814..66baccb 100644
--- a/sh/jshn.sh
+++ b/sh/jshn.sh
@@ -180,6 +180,10 @@ json_load() {
eval "`jshn -r "$1"`"
  }
  
+json_load_file() {

+   eval "`jshn -R "$1"`"
+}
+
  json_dump() {
jshn "$@" ${JSON_PREFIX:+-p "$JSON_PREFIX"} -w
  }



___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[Christian Beier]

Am Tue, 12 Dec 2017 19:39:48 +0100
schrieb Christian Beier :

> Am Tue, 12 Dec 2017 12:03:50 +0100
> schrieb John Crispin :
> 
> > Hi Christian,
> > 
> > small formatting nitpick inline ...  
> 
> Hi John,
> 
> Re-sending with tab adjustments and a space between if and the opening paren.
> 
> Cheers, 
> 
>Christian

Hi John,

Is there anything I forgot to take care of? I did not see this appear in
https://git.lede-project.org/?p=project/libubox.git yet. Or has the repo
location changed?

Cheers,

   Christian


pgpBXWhOJnn6k.pgp
Description: Digitale Signatur von OpenPGP
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[Christian Beier via Lede-dev]

The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.--- Begin Message ---
Am Tue, 12 Dec 2017 12:03:50 +0100
schrieb John Crispin :

> Hi Christian,
> 
> small formatting nitpick inline ...

Hi John,

Re-sending with tab adjustments and a space between if and the opening paren.

Cheers, 

   Christian
>From fcb002d7c8db41b27713f69422a4e2baecad08b0 Mon Sep 17 00:00:00 2001
From: Christian Beier 
Date: Tue, 12 Dec 2017 19:33:12 +0100
Subject: [PATCH] jshn: add functionality to read big JSON

The existing read functionality feeds the complete JSON to jshn as a
cmdline argument, leading to `-ash: jshn: Argument list too long`
errors for JSONs bigger than ca. 100KB.

This commit adds the ability to read the JSON directly from a file if
wanted, removing this shell-imposed size limit.

Tested on x86-64 and ar71xx. An mmap()-based solution was also evaluated,
but found to make no performance difference on either platform.

Signed-off-by: Christian Beier 
---
 jshn.c | 30 --
 sh/jshn.sh |  4 
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/jshn.c b/jshn.c
index 79136dd..4a69994 100644
--- a/jshn.c
+++ b/jshn.c
@@ -25,6 +25,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "list.h"
 
 #include "avl.h"
@@ -300,7 +302,7 @@ out:
 
 static int usage(const char *progname)
 {
-	fprintf(stderr, "Usage: %s [-n] [-i] -r |-w\n", progname);
+	fprintf(stderr, "Usage: %s [-n] [-i] -r |-R |-w\n", progname);
 	return 2;
 }
 
@@ -333,6 +335,10 @@ int main(int argc, char **argv)
 	struct env_var *vars;
 	int i;
 	int ch;
+	int fd;
+	struct stat sb;
+	char *fbuf;
+	int ret;
 
 	avl_init(_vars, avl_strcmp_var, false, NULL);
 	for (i = 0; environ[i]; i++);
@@ -354,7 +360,7 @@ int main(int argc, char **argv)
 		avl_insert(_vars, [i].avl);
 	}
 
-	while ((ch = getopt(argc, argv, "p:nir:w")) != -1) {
+	while ((ch = getopt(argc, argv, "p:nir:R:w")) != -1) {
 		switch(ch) {
 		case 'p':
 			var_prefix = optarg;
@@ -362,6 +368,26 @@ int main(int argc, char **argv)
 			break;
 		case 'r':
 			return jshn_parse(optarg);
+		case 'R':
+			if ((fd = open(optarg, O_RDONLY)) == -1) {
+fprintf(stderr, "Error opening %s\n", optarg);
+return 3;
+			}
+			if (fstat(fd, ) == -1) {
+fprintf(stderr, "Error getting size of %s\n", optarg);
+close(fd);
+return 3;
+			}
+			if (!(fbuf = malloc(sb.st_size)) || read(fd, fbuf, sb.st_size) != sb.st_size) {
+fprintf(stderr, "Error reading %s\n", optarg);
+free(fbuf);
+close(fd);
+return 3;
+			}
+			ret = jshn_parse(fbuf);
+			free(fbuf);
+			close(fd);
+			return ret;
 		case 'w':
 			return jshn_format(no_newline, indent);
 		case 'n':
diff --git a/sh/jshn.sh b/sh/jshn.sh
index bf76edb..0a146e1 100644
--- a/sh/jshn.sh
+++ b/sh/jshn.sh
@@ -174,6 +174,10 @@ json_load() {
 	eval "`jshn -r "$1"`"
 }
 
+json_load_file() {
+	eval "`jshn -R "$1"`"
+}
+
 json_dump() {
 	jshn "$@" ${JSON_PREFIX:+-p "$JSON_PREFIX"} -w 
 }
-- 
2.11.0

--- End Message ---
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[John Crispin]

Hi Christian,

small formatting nitpick inline ...


On 16/11/17 19:52, Christian Beier wrote:

The existing read functionality feeds the complete JSON to jshn as a
cmdline argument, leading to `-ash: jshn: Argument list too long`
errors for JSONs bigger than ca. 100KB.

This commit adds the ability to read the JSON directly from a file if
wanted, removing this shell-imposed size limit.

Tested on x86-64 and ar71xx. An mmap()-based solution was also evaluated,
but found to make no performance difference on either platform.

Signed-off-by: Christian Beier 
---
  jshn.c | 30 --
  sh/jshn.sh |  4 
  2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/jshn.c b/jshn.c
index 79136dd..a3866a0 100644
--- a/jshn.c
+++ b/jshn.c
@@ -25,6 +25,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 
  #include "list.h"
  
  #include "avl.h"

@@ -300,7 +302,7 @@ out:
  
  static int usage(const char *progname)

  {
-   fprintf(stderr, "Usage: %s [-n] [-i] -r |-w\n", progname);
+   fprintf(stderr, "Usage: %s [-n] [-i] -r |-R |-w\n", 
progname);
return 2;
  }
  
@@ -333,6 +335,10 @@ int main(int argc, char **argv)

struct env_var *vars;
int i;
int ch;
+   int fd;
+   struct stat sb;
+   char *fbuf;
+   int ret;
  
  	avl_init(_vars, avl_strcmp_var, false, NULL);

for (i = 0; environ[i]; i++);
@@ -354,7 +360,7 @@ int main(int argc, char **argv)
avl_insert(_vars, [i].avl);
}
  
-	while ((ch = getopt(argc, argv, "p:nir:w")) != -1) {

+   while ((ch = getopt(argc, argv, "p:nir:R:w")) != -1) {
switch(ch) {
case 'p':
var_prefix = optarg;
@@ -362,6 +368,26 @@ int main(int argc, char **argv)
break;
case 'r':
return jshn_parse(optarg);
+   case 'R':
+   if((fd = open(optarg, O_RDONLY)) == -1) {
 you have spaces leading up to the if() and then needs to be a space 
between if and (



+   fprintf(stderr, "Error opening %s\n", optarg);
+   return 3;

spaces vs tabs

+   }
+   if (fstat(fd, ) == -1) {
+   fprintf(stderr, "Error getting size of %s\n", 
optarg);
+   close(fd);
+   return 3;

spaces vs tabs

+   }
+   if(!(fbuf = malloc(sb.st_size)) || read(fd, fbuf, 
sb.st_size) != sb.st_size) {
+   fprintf(stderr, "Error reading %s\n", optarg);
+   free(fbuf);
+   close(fd);
+   return 3;

spaces vs tabs

    John


+   }
+   ret = jshn_parse(fbuf);
+   free(fbuf);
+   close(fd);
+   return ret;
case 'w':
return jshn_format(no_newline, indent);
case 'n':
diff --git a/sh/jshn.sh b/sh/jshn.sh
index bf76edb..0a146e1 100644
--- a/sh/jshn.sh
+++ b/sh/jshn.sh
@@ -174,6 +174,10 @@ json_load() {
eval "`jshn -r "$1"`"
  }
  
+json_load_file() {

+   eval "`jshn -R "$1"`"
+}
+
  json_dump() {
jshn "$@" ${JSON_PREFIX:+-p "$JSON_PREFIX"} -w
  }



___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[Alexandru Ardelean]

On Sat, Nov 18, 2017 at 3:55 PM, Christian Beier  wrote:
> Am Fri, 17 Nov 2017 09:17:39 +0200
> schrieb Alexandru Ardelean :
>
>> On Thu, Nov 16, 2017 at 8:52 PM, Christian Beier 
>> wrote:
>> > The existing read functionality feeds the complete JSON to jshn as a
>> > cmdline argument, leading to `-ash: jshn: Argument list too long`
>> > errors for JSONs bigger than ca. 100KB.
>> >
>> > This commit adds the ability to read the JSON directly from a file if
>> > wanted, removing this shell-imposed size limit.
>> >
>> > Tested on x86-64 and ar71xx. An mmap()-based solution was also evaluated,
>> > but found to make no performance difference on either platform.
>> >
>> > Signed-off-by: Christian Beier 
>> > ---
>> >  jshn.c | 30 --
>> >  sh/jshn.sh |  4 
>> >  2 files changed, 32 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/jshn.c b/jshn.c
>> > index 79136dd..a3866a0 100644
>> > --- a/jshn.c
>> > +++ b/jshn.c
>> > @@ -25,6 +25,8 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> > +#include 
>> >  #include "list.h"
>> >
>> >  #include "avl.h"
>> > @@ -300,7 +302,7 @@ out:
>> >
>> >  static int usage(const char *progname)
>> >  {
>> > -   fprintf(stderr, "Usage: %s [-n] [-i] -r |-w\n", progname);
>> > +   fprintf(stderr, "Usage: %s [-n] [-i] -r |-R |-w\n",
>> > progname); return 2;
>> >  }
>> >
>> > @@ -333,6 +335,10 @@ int main(int argc, char **argv)
>> > struct env_var *vars;
>> > int i;
>> > int ch;
>> > +   int fd;
>> > +   struct stat sb;
>> > +   char *fbuf;
>> > +   int ret;
>> >
>> > avl_init(_vars, avl_strcmp_var, false, NULL);
>> > for (i = 0; environ[i]; i++);
>> > @@ -354,7 +360,7 @@ int main(int argc, char **argv)
>> > avl_insert(_vars, [i].avl);
>> > }
>> >
>> > -   while ((ch = getopt(argc, argv, "p:nir:w")) != -1) {
>> > +   while ((ch = getopt(argc, argv, "p:nir:R:w")) != -1) {
>> > switch(ch) {
>> > case 'p':
>> > var_prefix = optarg;
>> > @@ -362,6 +368,26 @@ int main(int argc, char **argv)
>> > break;
>> > case 'r':
>> > return jshn_parse(optarg);
>> > +   case 'R':
>> > +   if((fd = open(optarg, O_RDONLY)) == -1) {
>> > +   fprintf(stderr, "Error opening %s\n", optarg);
>> > +   return 3;
>> > +   }
>> > +   if (fstat(fd, ) == -1) {
>> > +   fprintf(stderr, "Error getting size of %s\n",
>> > optarg);
>> > +   close(fd);
>> > +   return 3;
>> > +   }
>> > +   if(!(fbuf = malloc(sb.st_size)) || read(fd, fbuf,
>> > sb.st_size) != sb.st_size) {
>> > +   fprintf(stderr, "Error reading %s\n", optarg);
>> > +   free(fbuf);
>> > +   close(fd);
>> > +   return 3;
>> > +   }
>> > +   ret = jshn_parse(fbuf);
>> > +   free(fbuf);
>> > +   close(fd);
>> > +   return ret;
>> nitpick/preference: I would move this code into a file, so that the
>> case statement is not too loaded
>> it would allow for a simpler/cleaner code-path with some goto
>> statements [if put into a function]
>
> Yeah, I tried keeping all those error case printf and return blocks as minimal
> as possible, though it still boils down to 3. Might be an idea to factor out 
> the
> file-opening-and-reading into a separate function to keep the switch statement
> cleaner, but then OTOH it'd be a function with only one caller.
>
> Also, I was unsure if it'd be overkill to assign different exit codes to the
> different error conditions, so I simply used the first unused one (3) for all
> of them.
>
>>
>> > case 'w':
>> > return jshn_format(no_newline, indent);
>> > case 'n':
>> > diff --git a/sh/jshn.sh b/sh/jshn.sh
>> > index bf76edb..0a146e1 100644
>> > --- a/sh/jshn.sh
>> > +++ b/sh/jshn.sh
>> > @@ -174,6 +174,10 @@ json_load() {
>> > eval "`jshn -r "$1"`"
>> >  }
>> >
>> > +json_load_file() {
>> > +   eval "`jshn -R "$1"`"
>> would it be an idea to try to use $JSON_PREFIX here ?
>> for cases when you need to change the JSON namespace, the JSON_PREFIX
>> var is used if available
>> it would definitely complete this function
>
> TBH, I don't really know what JSON_PREFIX is for. I simply used the existing
> json_load() function as a blueprint.

oh right ;
disregard what i was saying ;
i was looking at the json_dump() function

JSON_PREFIX is used for changing the namespace;
that allows to load 

Re: [LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[Alexandru Ardelean]

On Thu, Nov 16, 2017 at 8:52 PM, Christian Beier  wrote:
> The existing read functionality feeds the complete JSON to jshn as a
> cmdline argument, leading to `-ash: jshn: Argument list too long`
> errors for JSONs bigger than ca. 100KB.
>
> This commit adds the ability to read the JSON directly from a file if
> wanted, removing this shell-imposed size limit.
>
> Tested on x86-64 and ar71xx. An mmap()-based solution was also evaluated,
> but found to make no performance difference on either platform.
>
> Signed-off-by: Christian Beier 
> ---
>  jshn.c | 30 --
>  sh/jshn.sh |  4 
>  2 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/jshn.c b/jshn.c
> index 79136dd..a3866a0 100644
> --- a/jshn.c
> +++ b/jshn.c
> @@ -25,6 +25,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include "list.h"
>
>  #include "avl.h"
> @@ -300,7 +302,7 @@ out:
>
>  static int usage(const char *progname)
>  {
> -   fprintf(stderr, "Usage: %s [-n] [-i] -r |-w\n", progname);
> +   fprintf(stderr, "Usage: %s [-n] [-i] -r |-R |-w\n", 
> progname);
> return 2;
>  }
>
> @@ -333,6 +335,10 @@ int main(int argc, char **argv)
> struct env_var *vars;
> int i;
> int ch;
> +   int fd;
> +   struct stat sb;
> +   char *fbuf;
> +   int ret;
>
> avl_init(_vars, avl_strcmp_var, false, NULL);
> for (i = 0; environ[i]; i++);
> @@ -354,7 +360,7 @@ int main(int argc, char **argv)
> avl_insert(_vars, [i].avl);
> }
>
> -   while ((ch = getopt(argc, argv, "p:nir:w")) != -1) {
> +   while ((ch = getopt(argc, argv, "p:nir:R:w")) != -1) {
> switch(ch) {
> case 'p':
> var_prefix = optarg;
> @@ -362,6 +368,26 @@ int main(int argc, char **argv)
> break;
> case 'r':
> return jshn_parse(optarg);
> +   case 'R':
> +   if((fd = open(optarg, O_RDONLY)) == -1) {
> +   fprintf(stderr, "Error opening %s\n", optarg);
> +   return 3;
> +   }
> +   if (fstat(fd, ) == -1) {
> +   fprintf(stderr, "Error getting size of %s\n", 
> optarg);
> +   close(fd);
> +   return 3;
> +   }
> +   if(!(fbuf = malloc(sb.st_size)) || read(fd, fbuf, 
> sb.st_size) != sb.st_size) {
> +   fprintf(stderr, "Error reading %s\n", optarg);
> +   free(fbuf);
> +   close(fd);
> +   return 3;
> +   }
> +   ret = jshn_parse(fbuf);
> +   free(fbuf);
> +   close(fd);
> +   return ret;
nitpick/preference: I would move this code into a file, so that the
case statement is not too loaded
it would allow for a simpler/cleaner code-path with some goto
statements [if put into a function]

> case 'w':
> return jshn_format(no_newline, indent);
> case 'n':
> diff --git a/sh/jshn.sh b/sh/jshn.sh
> index bf76edb..0a146e1 100644
> --- a/sh/jshn.sh
> +++ b/sh/jshn.sh
> @@ -174,6 +174,10 @@ json_load() {
> eval "`jshn -r "$1"`"
>  }
>
> +json_load_file() {
> +   eval "`jshn -R "$1"`"
would it be an idea to try to use $JSON_PREFIX here ?
for cases when you need to change the JSON namespace, the JSON_PREFIX
var is used if available
it would definitely complete this function

> +}
> +
>  json_dump() {
> jshn "$@" ${JSON_PREFIX:+-p "$JSON_PREFIX"} -w
>  }
> --
> 2.11.0
>

from my side [as a simple reviewer], this looks pretty neat

thanks :)

Alex

>
> ___
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

[LEDE-DEV] [PATCH] jshn: add functionality to read big JSON

[Christian Beier]

The existing read functionality feeds the complete JSON to jshn as a
cmdline argument, leading to `-ash: jshn: Argument list too long`
errors for JSONs bigger than ca. 100KB.

This commit adds the ability to read the JSON directly from a file if
wanted, removing this shell-imposed size limit.

Tested on x86-64 and ar71xx. An mmap()-based solution was also evaluated,
but found to make no performance difference on either platform.

Signed-off-by: Christian Beier 
---
 jshn.c | 30 --
 sh/jshn.sh |  4 
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/jshn.c b/jshn.c
index 79136dd..a3866a0 100644
--- a/jshn.c
+++ b/jshn.c
@@ -25,6 +25,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "list.h"
 
 #include "avl.h"
@@ -300,7 +302,7 @@ out:
 
 static int usage(const char *progname)
 {
-   fprintf(stderr, "Usage: %s [-n] [-i] -r |-w\n", progname);
+   fprintf(stderr, "Usage: %s [-n] [-i] -r |-R |-w\n", 
progname);
return 2;
 }
 
@@ -333,6 +335,10 @@ int main(int argc, char **argv)
struct env_var *vars;
int i;
int ch;
+   int fd;
+   struct stat sb;
+   char *fbuf;
+   int ret;
 
avl_init(_vars, avl_strcmp_var, false, NULL);
for (i = 0; environ[i]; i++);
@@ -354,7 +360,7 @@ int main(int argc, char **argv)
avl_insert(_vars, [i].avl);
}
 
-   while ((ch = getopt(argc, argv, "p:nir:w")) != -1) {
+   while ((ch = getopt(argc, argv, "p:nir:R:w")) != -1) {
switch(ch) {
case 'p':
var_prefix = optarg;
@@ -362,6 +368,26 @@ int main(int argc, char **argv)
break;
case 'r':
return jshn_parse(optarg);
+   case 'R':
+   if((fd = open(optarg, O_RDONLY)) == -1) {
+   fprintf(stderr, "Error opening %s\n", optarg);
+   return 3;
+   }
+   if (fstat(fd, ) == -1) {
+   fprintf(stderr, "Error getting size of %s\n", 
optarg);
+   close(fd);
+   return 3;
+   }
+   if(!(fbuf = malloc(sb.st_size)) || read(fd, fbuf, 
sb.st_size) != sb.st_size) {
+   fprintf(stderr, "Error reading %s\n", optarg);
+   free(fbuf);
+   close(fd);
+   return 3;
+   }
+   ret = jshn_parse(fbuf);
+   free(fbuf);
+   close(fd);
+   return ret;
case 'w':
return jshn_format(no_newline, indent);
case 'n':
diff --git a/sh/jshn.sh b/sh/jshn.sh
index bf76edb..0a146e1 100644
--- a/sh/jshn.sh
+++ b/sh/jshn.sh
@@ -174,6 +174,10 @@ json_load() {
eval "`jshn -r "$1"`"
 }
 
+json_load_file() {
+   eval "`jshn -R "$1"`"
+}
+
 json_dump() {
jshn "$@" ${JSON_PREFIX:+-p "$JSON_PREFIX"} -w 
 }
-- 
2.11.0


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev