Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface

2017-11-13 Thread Selva
Hi, On Mon, Nov 13, 2017 at 3:40 PM, David Sommerseth wrote: > On 13/11/17 14:44, François Kooman wrote: > > On 11/13/2017 01:16 PM, David Sommerseth wrote: > [...snip...] > >> But we should consider if we want to make use of a JSON library > >> producing the

Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface

2017-11-13 Thread David Sommerseth
On 13/11/17 14:44, François Kooman wrote: > On 11/13/2017 01:16 PM, David Sommerseth wrote: [...snip...] >> But we should consider if we want to make use of a JSON library >> producing the JSON streams. The reason is to ensure the output is >> according to the specification and that escaping if

Re: [Openvpn-devel] [PATCH v2] openvpnserv: Review MSVC down-casting warnings

2017-11-13 Thread Selva
Hi, Thanks for the v2 On Mon, Nov 13, 2017 at 4:49 AM, Simon Rozman wrote: > Data size arithmetic was reviewed according to 64-bit MSVC complaints. > > The warnings were addressed by migrating to size_t, rewriting the code, > or silencing the warnings by an explicit cast where

Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface

2017-11-13 Thread François Kooman
On 11/13/2017 01:16 PM, David Sommerseth wrote: > CSV just looks structured - until the layout is changed. While JSON > (and even XML for that matter, _not_ saying we should have XML support!) > is not strictly bound to the order of fields, so adding new fields is a > no-brainer with JSON

Re: [Openvpn-devel] [PATCH 12/13] Memory size arithmetic reviewed according to 64-bit MSVC complaints

2017-11-13 Thread Selva
Hi, > > const char *p = _tcsrchr(find->cFileName, TEXT('.')); > > return p && p != find->cFileName && !_tcsicmp(p+1, ext); > > > > No _tcslen() and no offending size variables. > > And that convoluted original is then gone. > > The resulting code would look a lot nicer indeed. Stay tuned

Re: [Openvpn-devel] [PATCHv2 7/7] Add gc_arena to struct argv to save allocations

2017-11-13 Thread David Sommerseth
On 12/11/17 09:34, Heiko Hund wrote: > With the private gc_arena we do not have to allocate the strings > found during parsing again, since we know the arena they are > allocated in is valid as long as the argv vector is. > > Signed-off-by: Heiko Hund > --- >

Re: [Openvpn-devel] [PATCHv2 6/7] argv: do fewer memory re-allocations

2017-11-13 Thread David Sommerseth
On 12/11/17 09:27, Heiko Hund wrote: > Prevent the re-allocations of memory when the internal argv grows > beyond 2 and 4 arguments by initially allocating argv to hold up to > 7 (+ trailing NULL) pointers. > > While at it rename argv_reset to argv_free to actually express > what's going on. Redo

Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface

2017-11-13 Thread David Sommerseth
On 13/11/17 11:15, Steffan Karger wrote: [...snip...] >>> You know, the funny thing is that my main reason for implementing JSON >>> support for status was to have it easily _machine_ readable, for higher >>> level programming languages where JSON is "first class citizen", and not >>> primarily

Re: [Openvpn-devel] [PATCHv2 5/7] re-implement argv_printf_*()

2017-11-13 Thread David Sommerseth
Hey, Thanks a lot for putting a renewed effort into this. I've reviewed this and compile tested with GCC 4.8.5, 6.3.1 and 7.2.1 - as there are some warnings appearing; I'll come back to them in a bit. On 12/11/17 09:24, Heiko Hund wrote: > The previous implementation had the problem that it

Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface

2017-11-13 Thread Steffan Karger
Hi, Since I don't use the mgmt interface / status file a lot, I didn't chip in earlier. But since this quite closely matches my initial thoughts I'll do so anyway. On 13-11-17 10:54, Antonio Quartulli wrote: > On 13/11/17 17:44, François Kooman wrote: >> On 11/12/2017 10:42 PM, Gert Doering

Re: [Openvpn-devel] [PATCH] Add generated openvpn.doxyfile to .gitignore

2017-11-13 Thread Antonio Quartulli
On 12/11/17 01:14, Gert van Dijk wrote: > I think this was omitted in 66bf378e. > > Signed-off-by: Gert van Dijk Acked-by: Antonio Quartulli We definitely need this otherwise the doxyfile won't leave you alone. Thanks! -- Antonio Quartulli

Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface

2017-11-13 Thread Antonio Quartulli
Hi, On 13/11/17 17:44, François Kooman wrote: > On 11/12/2017 10:42 PM, Gert Doering wrote: >> On Sun, Nov 12, 2017 at 12:21:06PM -0500, Selva wrote: >>> One of the niceties of JSON is its readability which is greatly reduced >>> if formatted without line breaks. >> >> +1 >> >> the difference in

[Openvpn-devel] [PATCH v2] openvpnserv: Review MSVC down-casting warnings

2017-11-13 Thread Simon Rozman
Data size arithmetic was reviewed according to 64-bit MSVC complaints. The warnings were addressed by migrating to size_t, rewriting the code, or silencing the warnings by an explicit cast where appropriate. --- src/openvpnserv/automatic.c | 20

Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface

2017-11-13 Thread François Kooman
On 11/12/2017 10:42 PM, Gert Doering wrote: > On Sun, Nov 12, 2017 at 12:21:06PM -0500, Selva wrote: >> One of the niceties of JSON is its readability which is greatly reduced >> if formatted without line breaks. > > +1 > > the difference in efficienty is not large, but "human readability for >

Re: [Openvpn-devel] [PATCH 12/13] Memory size arithmetic reviewed according to 64-bit MSVC complaints

2017-11-13 Thread Simon Rozman
Hi, > Some of these changes are of dubious value as the string lengths involved > are guaranteed to be small and there is no scope for overflow. And casting > only stops the compiler warning, not potential overflow, if any.. Exactly. Where there's no scope for an overflow and compiler is too