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 <heiko.h...@sophos.com>
> ---
> 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 419b1dc6..51dd1b22 100644
> --- a/src/openvpn/argv.c
> +++ b/src/openvpn/argv.c
> @@ -46,12 +46,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, &a->gc);
Ahh! Here it is :) Disregard my comment about this in PATCH v2 6/7 :)
> for (i = 0; i < a->argc; ++i)
> {
> newargv[i] = a->argv[i];
> }
> - free(a->argv);
> a->argv = newargv;
> a->capacity = newcap;
> }
> @@ -63,6 +62,7 @@ argv_init(struct argv *a)
> a->capacity = 0;
> a->argc = 0;
> a->argv = NULL;
> + a->gc = gc_new();
> argv_extend(a, 8);
> }
>
> @@ -77,24 +77,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(&a->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
> @@ -106,7 +103,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 (i = 0; i < a->argc; ++i)
> {
> - argv_append(&r, string_alloc(a->argv[i], NULL));
> + argv_append(&r, string_alloc(a->argv[i], &r.gc));
> }
> }
> 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, &r.gc);
> return r;
> }
>
> @@ -243,7 +240,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, &gc);
> + buf = gc_malloc(size, false, &a->gc);
> len = vsnprintf(buf, size, f, arglist);
> if (len < 0 || len >= size)
> {
> @@ -255,11 +252,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)
> {
> @@ -303,23 +300,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, &gc);
> + nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
> D_ARGV_PARSE_CMD, &a->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, &a->gc));
> }
> -
> - gc_free(&gc);
> }
> diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
> index 2a1945e3..a1aa7ebb 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 e15e2fe5..5bfef8af 100644
> --- a/tests/unit_tests/openvpn/test_argv.c
> +++ b/tests/unit_tests/openvpn/test_argv.c
> @@ -118,6 +118,28 @@ argv_printf__empty_parameter__argc_correct(void **state)
> }
>
> 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=looooooooooooooooooooooooooooooooooooooooooooooooong",
> +
> "--long-cat=loooooooooooooooooooooooooooooooooooooooooooooooooooonger",
> +
> "file_with_very_descriptive_filename_that_leaves_no_questions_open.jpg.exe"
> + };
> +
> + argv_printf(&a, "%s %s %s %s", args[0], args[1], args[2], args[3]);
> + assert_int_equal(a.argc, 4);
> + for (i = 0; i < a.argc; i++)
> + {
> + assert_string_equal(a.argv[i], args[i]);
> + }
> +
> + argv_free(&a);
> +}
> +
> +static void
> argv_parse_cmd__command_string__argc_correct(void **state)
> {
> struct argv a = argv_new();
> @@ -233,6 +255,7 @@ main(void)
> cmocka_unit_test(argv_printf__group_sep_in_arg__fail_no_ouput),
>
> cmocka_unit_test(argv_printf__combined_path_with_spaces__argc_correct),
> cmocka_unit_test(argv_printf__empty_parameter__argc_correct),
> + cmocka_unit_test(argv_printf__long_args__data_correct),
> cmocka_unit_test(argv_parse_cmd__command_string__argc_correct),
>
> cmocka_unit_test(argv_parse_cmd__command_and_extra_options__argc_correct),
> cmocka_unit_test(argv_printf_cat__used_twice__argc_correct),
>
Only glared at these changes. They make sense and the Acked-By is not
far away at all! Just want to run some build tests with the other final
patches in place first.
--
kind regards,
David Sommerseth
OpenVPN, Inc
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
