Re: [Openvpn-devel] [PATCH 3/4] Add gc_arena to struct argv to save allocations

2020-02-12 Thread Arne Schwabe 
Am 06.02.20 um 14:21 schrieb David Sommerseth:
> From: Heiko Hund 
> 
> 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 
> Signed-off-by: David Sommerseth 

Acked-By: Arne Schwabe 

I don't feel it is really necessary but it is slight improvement and
this been too many revisions already, so get it in.





signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 3/4] Add gc_arena to struct argv to save allocations

2020-02-06 Thread David Sommerseth 
From: Heiko Hund 

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 
Signed-off-by: David Sommerseth 
---
 src/openvpn/argv.c   | 50 
 src/openvpn/argv.h   |  1 +
 tests/unit_tests/openvpn/test_argv.c | 23 +
 3 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index 4f7aa4e5..7d949d24 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -47,12 +47,11 @@ argv_extend(struct argv *a, const size_t newcap)
 {
 char **newargv;
 size_t i;
-ALLOC_ARRAY_CLEAR(newargv, char *, newcap);
+ALLOC_ARRAY_CLEAR_GC(newargv, char *, newcap, >gc);
 for (i = 0; i < a->argc; ++i)
 {
 newargv[i] = a->argv[i];
 }
-free(a->argv);
 a->argv = newargv;
 a->capacity = newcap;
 }
@@ -64,6 +63,7 @@ argv_init(struct argv *a)
 a->capacity = 0;
 a->argc = 0;
 a->argv = NULL;
+a->gc = gc_new();
 argv_extend(a, 8);
 }
 
@@ -78,24 +78,21 @@ argv_new(void)
 void
 argv_free(struct argv *a)
 {
-size_t i;
-for (i = 0; i < a->argc; ++i)
-{
-free(a->argv[i]);
-}
-free(a->argv);
+gc_free(>gc);
 }
 
 static void
 argv_reset(struct argv *a)
 {
-size_t i;
-for (i = 0; i < a->argc; ++i)
+if (a->argc)
 {
-free(a->argv[i]);
-a->argv[i] = NULL;
+size_t i;
+for (i = 0; i < a->argc; ++i)
+{
+a->argv[i] = NULL;
+}
+a->argc = 0;
 }
-a->argc = 0;
 }
 
 static void
@@ -107,7 +104,7 @@ argv_grow(struct argv *a, const size_t add)
 }
 
 static void
-argv_append(struct argv *a, char *str)  /* str must have been malloced or be 
NULL */
+argv_append(struct argv *a, char *str)  /* str must have been gc_malloced or 
be NULL */
 {
 argv_grow(a, 1);
 a->argv[a->argc++] = str;
@@ -127,7 +124,7 @@ argv_clone(const struct argv *a, const size_t headroom)
 {
 for (size_t i = 0; i < a->argc; ++i)
 {
-argv_append(, string_alloc(a->argv[i], NULL));
+argv_append(, string_alloc(a->argv[i], ));
 }
 }
 return r;
@@ -138,7 +135,7 @@ argv_insert_head(const struct argv *a, const char *head)
 {
 struct argv r;
 r = argv_clone(a, 1);
-r.argv[0] = string_alloc(head, NULL);
+r.argv[0] = string_alloc(head, );
 return r;
 }
 
@@ -222,7 +219,6 @@ argv_prep_format(const char *format, const char delim, 
size_t *count, struct gc_
 static bool
 argv_printf_arglist(struct argv *a, const char *format, va_list arglist)
 {
-struct gc_arena gc = gc_new();
 const char delim = 0x1D;  /* ASCII Group Separator (GS) */
 bool res = false;
 
@@ -236,7 +232,7 @@ argv_printf_arglist(struct argv *a, const char *format, 
va_list arglist)
  *
  */
 size_t argc = a->argc;
-char *f = argv_prep_format(format, delim, , );
+char *f = argv_prep_format(format, delim, , >gc);
 if (f == NULL)
 {
 goto out;
@@ -256,8 +252,8 @@ argv_printf_arglist(struct argv *a, const char *format, 
va_list arglist)
  *  Do the actual vsnprintf() operation, which expands the format
  *  string with the provided arguments.
  */
-size_t size = len + 1;
-char *buf = gc_malloc(size, false, );
+size_t size = adjust_power_of_2(len + 1);
+char *buf = gc_malloc(size, false, >gc);
 len = vsnprintf(buf, size, f, arglist);
 if (len < 0 || len >= size)
 {
@@ -272,11 +268,11 @@ argv_printf_arglist(struct argv *a, const char *format, 
va_list arglist)
 while (end)
 {
 *end = '\0';
-argv_append(a, string_alloc(buf, NULL));
+argv_append(a, buf);
 buf = end + 1;
 end = strchr(buf, delim);
 }
-argv_append(a, string_alloc(buf, NULL));
+argv_append(a, buf);
 
 if (a->argc != argc)
 {
@@ -287,7 +283,6 @@ argv_printf_arglist(struct argv *a, const char *format, 
va_list arglist)
 res = true;
 
 out:
-gc_free();
 return res;
 }
 
@@ -321,21 +316,18 @@ argv_parse_cmd(struct argv *a, const char *s)
 {
 argv_reset(a);
 
-struct gc_arena gc = gc_new();
 char *parms[MAX_PARMS + 1] = { 0 };
-int nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0, 
D_ARGV_PARSE_CMD, );
+int nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0, 
D_ARGV_PARSE_CMD, >gc);
 if (nparms)
 {
 int i;
 for (i = 0; i < nparms; ++i)
 {
-argv_append(a, string_alloc(parms[i], NULL));
+argv_append(a, parms[i]);
 }
 }
 else
 {
-argv_append(a, string_alloc(s, NULL));
+argv_append(a, string_alloc(s, >gc));
 }
-
-gc_free();
 }
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index

Re: [Openvpn-devel] [PATCH 3/4] Add gc_arena to struct argv to save allocations

2018-10-24 Thread David Sommerseth 
On 19/10/18 17:56, David Sommerseth wrote:
> From: Heiko Hund 
> 
> 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 
> ---
>  src/openvpn/argv.c   | 44 
>  src/openvpn/argv.h   |  1 +
>  tests/unit_tests/openvpn/test_argv.c | 23 +++
>  3 files changed, 43 insertions(+), 25 deletions(-)

This is a simple rebase of [0, 1] on top of the previous two patches.  Nothing
more exciting to say about this.  My last review didn't have any further
remarks than "will test it"; which I've done :)

[0] 
[1]



-- 
kind regards,

David Sommerseth
OpenVPN Inc




signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 3/4] Add gc_arena to struct argv to save allocations

2018-10-19 Thread David Sommerseth 
From: Heiko Hund 

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 
---
 src/openvpn/argv.c   | 44 
 src/openvpn/argv.h   |  1 +
 tests/unit_tests/openvpn/test_argv.c | 23 +++
 3 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index ff6d0b46..39c6fa9e 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -47,12 +47,11 @@ argv_extend(struct argv *a, const size_t newcap)
 {
 char **newargv;
 size_t i;
-ALLOC_ARRAY_CLEAR(newargv, char *, newcap);
+ALLOC_ARRAY_CLEAR_GC(newargv, char *, newcap, >gc);
 for (i = 0; i < a->argc; ++i)
 {
 newargv[i] = a->argv[i];
 }
-free(a->argv);
 a->argv = newargv;
 a->capacity = newcap;
 }
@@ -64,6 +63,7 @@ argv_init(struct argv *a)
 a->capacity = 0;
 a->argc = 0;
 a->argv = NULL;
+a->gc = gc_new();
 argv_extend(a, 8);
 }
 
@@ -78,24 +78,21 @@ argv_new(void)
 void
 argv_free(struct argv *a)
 {
-size_t i;
-for (i = 0; i < a->argc; ++i)
-{
-free(a->argv[i]);
-}
-free(a->argv);
+gc_free(>gc);
 }
 
 static void
 argv_reset(struct argv *a)
 {
-size_t i;
-for (i = 0; i < a->argc; ++i)
+if (a->argc)
 {
-free(a->argv[i]);
-a->argv[i] = NULL;
+size_t i;
+for (i = 0; i < a->argc; ++i)
+{
+a->argv[i] = NULL;
+}
+a->argc = 0;
 }
-a->argc = 0;
 }
 
 static void
@@ -107,7 +104,7 @@ argv_grow(struct argv *a, const size_t add)
 }
 
 static void
-argv_append(struct argv *a, char *str)  /* str must have been malloced or be 
NULL */
+argv_append(struct argv *a, char *str)  /* str must have been gc_malloced or 
be NULL */
 {
 argv_grow(a, 1);
 a->argv[a->argc++] = str;
@@ -128,7 +125,7 @@ argv_clone(const struct argv *a, const size_t headroom)
 {
 for (i = 0; i < a->argc; ++i)
 {
-argv_append(, string_alloc(a->argv[i], NULL));
+argv_append(, string_alloc(a->argv[i], ));
 }
 }
 return r;
@@ -139,7 +136,7 @@ argv_insert_head(const struct argv *a, const char *head)
 {
 struct argv r;
 r = argv_clone(a, 1);
-r.argv[0] = string_alloc(head, NULL);
+r.argv[0] = string_alloc(head, );
 return r;
 }
 
@@ -251,7 +248,7 @@ argv_printf_arglist(struct argv *a, const char *format, 
va_list arglist)
 }
 
 size = adjust_power_of_2(len + 1);
-buf = gc_malloc(size, false, );
+buf = gc_malloc(size, false, >gc);
 len = vsnprintf(buf, size, f, arglist);
 if (len < 0 || len >= size)
 {
@@ -263,11 +260,11 @@ argv_printf_arglist(struct argv *a, const char *format, 
va_list arglist)
 while (end)
 {
 *end = '\0';
-argv_append(a, string_alloc(buf, NULL));
+argv_append(a, buf);
 buf = end + 1;
 end = strchr(buf, delim);
 }
-argv_append(a, string_alloc(buf, NULL));
+argv_append(a, buf);
 
 if (a->argc != argc)
 {
@@ -314,23 +311,20 @@ argv_parse_cmd(struct argv *a, const char *s)
 {
 int nparms;
 char *parms[MAX_PARMS + 1];
-struct gc_arena gc = gc_new();
 
 argv_reset(a);
 
-nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0, 
D_ARGV_PARSE_CMD, );
+nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0, 
D_ARGV_PARSE_CMD, >gc);
 if (nparms)
 {
 int i;
 for (i = 0; i < nparms; ++i)
 {
-argv_append(a, string_alloc(parms[i], NULL));
+argv_append(a, parms[i]);
 }
 }
 else
 {
-argv_append(a, string_alloc(s, NULL));
+argv_append(a, string_alloc(s, >gc));
 }
-
-gc_free();
 }
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index 989cd297..943c78ef 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -33,6 +33,7 @@
 #include "buffer.h"
 
 struct argv {
+struct gc_arena gc;
 size_t capacity;
 size_t argc;
 char **argv;
diff --git a/tests/unit_tests/openvpn/test_argv.c 
b/tests/unit_tests/openvpn/test_argv.c
index fde0ba45..25e80c1c 100644
--- a/tests/unit_tests/openvpn/test_argv.c
+++ b/tests/unit_tests/openvpn/test_argv.c
@@ -117,6 +117,28 @@ argv_printf__empty_parameter__argc_correct(void **state)
 argv_free();
 }
 
+static void
+argv_printf__long_args__data_correct(void **state)
+{
+int i;
+struct argv a = argv_new();
+const char *args[] = {
+"good_tools_have_good_names_even_though_it_might_impair_typing",
+"--long-opt=long",
+"--long-cat=lnger",
+
"file_with_very_descriptive_filename_that_leaves_no_questions_open.jpg.exe"