From patchwork Fri Oct 19 04:56:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Sommerseth X-Patchwork-Id: 561 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director12.mail.ord1d.rsapps.net ([172.27.255.56]) by backend30.mail.ord1d.rsapps.net with LMTP id OC6wG/7+yVtWZgAAIUCqbw for ; Fri, 19 Oct 2018 11:57:50 -0400 Received: from proxy5.mail.iad3a.rsapps.net ([172.27.255.56]) by director12.mail.ord1d.rsapps.net with LMTP id uMTdGP7+yVv9ZQAAIasKDg ; Fri, 19 Oct 2018 11:57:50 -0400 Received: from smtp49.gate.iad3a ([172.27.255.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy5.mail.iad3a.rsapps.net with LMTP id IG7cEv7+yVtZUQAAhn5joQ ; Fri, 19 Oct 2018 11:57:50 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp49.gate.iad3a.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=openvpn.net X-Suspicious-Flag: YES X-Classification-ID: ba5029c4-d3b7-11e8-b2f0-525400fffce0-1-1 Received: from [216.105.38.7] ([216.105.38.7:25759] helo=lists.sourceforge.net) by smtp49.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 26/B6-08400-DFEF9CB5; Fri, 19 Oct 2018 11:57:50 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1gDX99-00062L-U7; Fri, 19 Oct 2018 15:57:07 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1gDX98-00061m-5U for openvpn-devel@lists.sourceforge.net; Fri, 19 Oct 2018 15:57:06 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:Cc: To:From:Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=lj6Ofs0oBHyZkG1x5iL3htJRk8smQjEJG8F/zSwyuIQ=; b=l5rjntCMz+h3AUoNOMHsutGYFc 7FuimwO1OIIG1rqHBPQIegif897lvm1wFtje10nBHWofs/lHbaSEHUjNenIX1s90biNnsnmYfs66H WPR8DxNht4tSflU3U42zCVq0rbXnkLKA+cBRzRcCJqym81CGsuWxKR2rfyjAPGTnDvA0=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To :MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=lj6Ofs0oBHyZkG1x5iL3htJRk8smQjEJG8F/zSwyuIQ=; b=du0eDhICAIIcL2fLGkyVB+On6q y8b3EonQkr7TgTvZPI02O2RdQJbTKkNOL1jqloJN7+WjwITWRO6zf/S+fua7MGzxGFteqXXiG9C+c 0lPlDIHTSYatIR0zavI8gXtAvDFwWiaGEXOf6iHWs8i/U9M0YygljFmr22MgHYm00vKU=; Received: from mx0.basenordic.cloud ([185.212.44.139]) by sfi-mx-4.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) id 1gDX95-005A65-RT for openvpn-devel@lists.sourceforge.net; Fri, 19 Oct 2018 15:57:06 +0000 Received: from localhost (unknown [IPv6:::1]) by mx0.basenordic.cloud (Postfix) with ESMTP id 2F27282A39F; Fri, 19 Oct 2018 15:56:56 +0000 (UTC) Received: from mx0.basenordic.cloud ([127.0.0.1]) by localhost (winterfell.topphemmelig.net [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id MHhmivBIB0ps; Fri, 19 Oct 2018 17:56:53 +0200 (CEST) Received: from zimbra.sommerseth.email (zimbra.sommerseth.email [172.16.33.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx0.basenordic.cloud (Postfix) with ESMTPS id B56CC849D5E; Fri, 19 Oct 2018 17:56:50 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by zimbra.sommerseth.email (Postfix) with ESMTP id 49F5D51E21D9; Fri, 19 Oct 2018 17:56:50 +0200 (CEST) Received: from zimbra.sommerseth.email ([127.0.0.1]) by localhost (zimbra.sommerseth.email [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id EHFkRGTirWDS; Fri, 19 Oct 2018 17:56:50 +0200 (CEST) Received: from optimus.homebase.sommerseths.net (unknown [10.35.7.3]) by zimbra.sommerseth.email (Postfix) with ESMTPS id 0CEC951E21D8; Fri, 19 Oct 2018 17:56:50 +0200 (CEST) From: David Sommerseth To: openvpn-devel@lists.sourceforge.net Date: Fri, 19 Oct 2018 17:56:35 +0200 Message-Id: <20181019155638.9498-2-davids@openvpn.net> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181019155638.9498-1-davids@openvpn.net> References: <20181019155638.9498-1-davids@openvpn.net> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 SPF_HELO_PASS SPF: HELO matches SPF record 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different -0.0 SPF_PASS SPF: sender matches SPF record 0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1gDX95-005A65-RT Subject: [Openvpn-devel] [PATCH 1/4] re-implement argv_printf_*() X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox From: Heiko Hund The previous implementation had the problem that it was not fully compatible with printf() and could only detect % format directives following a space character (0x20). It modifies the format string and inserts marks to separate groups before passing it to the regular printf in libc. The marks are later used to separate the output string into individual command line arguments. The choice of 0x1D as the argument delimiter is based on the assumption that no "regular" string passed to argv_printf_*() will ever have to contain that byte (and the fact that it actually is the ASCII "group separator" control character, which fits its purpose). This commit has been updated by David Sommerseth based on his feedback on the mailing list discussions earlier on. Signed-off-by: Heiko Hund Signed-off-by: David Sommerseth --- src/openvpn/argv.c | 266 ++++++++++++--------------- src/openvpn/argv.h | 4 +- src/openvpn/route.c | 8 +- src/openvpn/tun.c | 24 +-- tests/unit_tests/openvpn/test_argv.c | 58 +++++- 5 files changed, 192 insertions(+), 168 deletions(-) diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c index 9100a196..9606f382 100644 --- a/src/openvpn/argv.c +++ b/src/openvpn/argv.c @@ -131,64 +131,6 @@ argv_insert_head(const struct argv *a, const char *head) return r; } -static char * -argv_term(const char **f) -{ - const char *p = *f; - const char *term = NULL; - size_t termlen = 0; - - if (*p == '\0') - { - return NULL; - } - - while (true) - { - const int c = *p; - if (c == '\0') - { - break; - } - if (term) - { - if (!isspace(c)) - { - ++termlen; - } - else - { - break; - } - } - else - { - if (!isspace(c)) - { - term = p; - termlen = 1; - } - } - ++p; - } - *f = p; - - if (term) - { - char *ret; - ASSERT(termlen > 0); - ret = malloc(termlen + 1); - check_malloc_return(ret); - memcpy(ret, term, termlen); - ret[termlen] = '\0'; - return ret; - } - else - { - return NULL; - } -} - const char * argv_str(const struct argv *a, struct gc_arena *gc, const unsigned int flags) { @@ -218,119 +160,151 @@ argv_msg_prefix(const int msglev, const struct argv *a, const char *prefix) gc_free(&gc); } -static void + +/* + * argv_prep_format - prepare argv format string for further processing + * + * Individual argument must be separated by space. Ignores leading and trailing spaces. + * Consecutive spaces count as one. Returns prepared format string, with space replaced + * by delim and adds the number of arguments to the count parameter. + */ +static char * +argv_prep_format(const char *format, const char delim, size_t *count, struct gc_arena *gc) +{ + int i, j; + char *f; + bool in_token = false; + + if (format == NULL) + { + return NULL; + } + + f = gc_malloc(strlen(format) + 1, true, gc); + for (i = 0, j = 0; i < strlen(format); i++) + { + if (format[i] == ' ') + { + in_token = false; + continue; + } + + if (!in_token) + { + ++*count; + + /* + * We don't add any delimiter to the resulting + * format string before something has been added there. + * The resulting format string will never start with a delimiter. + */ + if (j > 0) + { + f[j++] = delim; + } + } + + f[j++] = format[i]; + in_token = true; + } + + return f; +} + +/* + * argv_printf_arglist - create a struct argv from a format string + * + * Instead of parsing the format string ourselves place delimiters via argv_prep_format() + * before we let libc's printf() do the parsing. Then split the resulting string at the + * injected delimiters. + */ +static bool argv_printf_arglist(struct argv *a, const char *format, va_list arglist) { - char *term; - const char *f = format; + struct gc_arena gc = gc_new(); + const char delim = 0x1D; /* ASCII Group Separator (GS) */ + char *f, *buf, *end; + size_t argc, size; + bool res = false; + va_list tmplist; + int len; argv_extend(a, 1); /* ensure trailing NULL */ - while ((term = argv_term(&f)) != NULL) + argc = a->argc; + f = argv_prep_format(format, delim, &argc, &gc); + if (f == NULL) { - if (term[0] == '%') - { - if (!strcmp(term, "%s")) - { - char *s = va_arg(arglist, char *); - if (!s) - { - s = ""; - } - argv_append(a, string_alloc(s, NULL)); - } - else if (!strcmp(term, "%d")) - { - char numstr[64]; - openvpn_snprintf(numstr, sizeof(numstr), "%d", va_arg(arglist, int)); - argv_append(a, string_alloc(numstr, NULL)); - } - else if (!strcmp(term, "%u")) - { - char numstr[64]; - openvpn_snprintf(numstr, sizeof(numstr), "%u", va_arg(arglist, unsigned int)); - argv_append(a, string_alloc(numstr, NULL)); - } - else if (!strcmp(term, "%lu")) - { - char numstr[64]; - openvpn_snprintf(numstr, sizeof(numstr), "%lu", - va_arg(arglist, unsigned long)); - argv_append(a, string_alloc(numstr, NULL)); - } - else if (!strcmp(term, "%s/%d")) - { - char numstr[64]; - char *s = va_arg(arglist, char *); - - if (!s) - { - s = ""; - } + goto out; + } - openvpn_snprintf(numstr, sizeof(numstr), "%d", va_arg(arglist, int)); + /* determine minimum buffer size */ + va_copy(tmplist, arglist); + len = vsnprintf(NULL, 0, f, tmplist); + va_end(tmplist); + if (len < 0) + { + goto out; + } - { - const size_t len = strlen(s) + strlen(numstr) + 2; - char *combined = (char *) malloc(len); - check_malloc_return(combined); + size = adjust_power_of_2(len + 1); + buf = gc_malloc(size, false, &gc); + len = vsnprintf(buf, size, f, arglist); + if (len < 0 || len >= size) + { + goto out; + } - strcpy(combined, s); - strcat(combined, "/"); - strcat(combined, numstr); - argv_append(a, combined); - } - } - else if (!strcmp(term, "%s%sc")) - { - char *s1 = va_arg(arglist, char *); - char *s2 = va_arg(arglist, char *); - char *combined; + /* split the string at the delimiters */ + end = strchr(buf, delim); + while (end) + { + *end = '\0'; + argv_append(a, string_alloc(buf, NULL)); + buf = end + 1; + end = strchr(buf, delim); + } + argv_append(a, string_alloc(buf, NULL)); - if (!s1) - { - s1 = ""; - } - if (!s2) - { - s2 = ""; - } - combined = (char *) malloc(strlen(s1) + strlen(s2) + 1); - check_malloc_return(combined); - strcpy(combined, s1); - strcat(combined, s2); - argv_append(a, combined); - } - else - { - ASSERT(0); - } - free(term); - } - else - { - argv_append(a, term); - } + if (a->argc != argc) + { + /* Someone snuck in a GS (0x1D), fail gracefully */ + argv_reset(a); + argv_extend(a, 1); /* ensure trailing NULL */ + goto out; } + + res = true; + +out: + gc_free(&gc); + return res; } -void + + +bool argv_printf(struct argv *a, const char *format, ...) { + bool res; va_list arglist; - argv_reset(a); + va_start(arglist, format); - argv_printf_arglist(a, format, arglist); + argv_reset(a); + res = argv_printf_arglist(a, format, arglist); va_end(arglist); + return res; } -void +bool argv_printf_cat(struct argv *a, const char *format, ...) { + bool res; va_list arglist; va_start(arglist, format); - argv_printf_arglist(a, format, arglist); + res = argv_printf_arglist(a, format, arglist); va_end(arglist); + return res; } void diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h index 9d9f3873..b9105a43 100644 --- a/src/openvpn/argv.h +++ b/src/openvpn/argv.h @@ -52,7 +52,7 @@ void argv_msg_prefix(const int msglev, const struct argv *a, const char *prefix) void argv_parse_cmd(struct argv *a, const char *s); -void argv_printf(struct argv *a, const char *format, ...) +bool argv_printf(struct argv *a, const char *format, ...) #ifdef __GNUC__ #if __USE_MINGW_ANSI_STDIO __attribute__ ((format(gnu_printf, 2, 3))) @@ -62,7 +62,7 @@ __attribute__ ((format(__printf__, 2, 3))) #endif ; -void argv_printf_cat(struct argv *a, const char *format, ...) +bool argv_printf_cat(struct argv *a, const char *format, ...) #ifdef __GNUC__ #if __USE_MINGW_ANSI_STDIO __attribute__ ((format(gnu_printf, 2, 3))) diff --git a/src/openvpn/route.c b/src/openvpn/route.c index d97e8dba..9649d197 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -1614,7 +1614,7 @@ add_route(struct route_ipv4 *r, #elif defined (_WIN32) { DWORD ai = TUN_ADAPTER_INDEX_INVALID; - argv_printf(&argv, "%s%sc ADD %s MASK %s %s", + argv_printf(&argv, "%s%s ADD %s MASK %s %s", get_win_sys_path(), WIN_ROUTE_PATH_SUFFIX, network, @@ -1979,7 +1979,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag device = buf_bptr(&out); /* netsh interface ipv6 add route 2001:db8::/32 MyTunDevice */ - argv_printf(&argv, "%s%sc interface ipv6 add route %s/%d %s", + argv_printf(&argv, "%s%s interface ipv6 add route %s/%d %s", get_win_sys_path(), NETSH_PATH_SUFFIX, network, @@ -2186,7 +2186,7 @@ delete_route(struct route_ipv4 *r, #elif defined (_WIN32) - argv_printf(&argv, "%s%sc DELETE %s MASK %s %s", + argv_printf(&argv, "%s%s DELETE %s MASK %s %s", get_win_sys_path(), WIN_ROUTE_PATH_SUFFIX, network, @@ -2426,7 +2426,7 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, unsigned device = buf_bptr(&out); /* netsh interface ipv6 delete route 2001:db8::/32 MyTunDevice */ - argv_printf(&argv, "%s%sc interface ipv6 delete route %s/%d %s", + argv_printf(&argv, "%s%s interface ipv6 delete route %s/%d %s", get_win_sys_path(), NETSH_PATH_SUFFIX, network, diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 948fd17d..f80aedf8 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -994,7 +994,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu, openvpn_snprintf(iface, sizeof(iface), "interface=%lu", tt->adapter_index); - argv_printf(&argv, "%s%sc interface ipv6 set address %s %s store=active", + argv_printf(&argv, "%s%s interface ipv6 set address %s %s store=active", get_win_sys_path(), NETSH_PATH_SUFFIX, iface, ifconfig_ipv6_local); netsh_command(&argv, 4, M_FATAL); @@ -4873,14 +4873,14 @@ ipconfig_register_dns(const struct env_set *es) msg(D_TUNTAP_INFO, "Start ipconfig commands for register-dns..."); netcmd_semaphore_lock(); - argv_printf(&argv, "%s%sc /flushdns", + argv_printf(&argv, "%s%s /flushdns", get_win_sys_path(), WIN_IPCONFIG_PATH_SUFFIX); argv_msg(D_TUNTAP_INFO, &argv); status = openvpn_execve_check(&argv, es, 0, err); argv_reset(&argv); - argv_printf(&argv, "%s%sc /registerdns", + argv_printf(&argv, "%s%s /registerdns", get_win_sys_path(), WIN_IPCONFIG_PATH_SUFFIX); argv_msg(D_TUNTAP_INFO, &argv); @@ -4994,8 +4994,8 @@ netsh_set_dns6_servers(const struct in6_addr *addr_list, for (int i = 0; i < addr_len; ++i) { const char *fmt = (i == 0) ? - "%s%sc interface ipv6 set dns %s static %s" - : "%s%sc interface ipv6 add dns %s %s"; + "%s%s interface ipv6 set dns %s static %s" + : "%s%s interface ipv6 add dns %s %s"; argv_printf(&argv, fmt, get_win_sys_path(), NETSH_PATH_SUFFIX, flex_name, print_in6_addr(addr_list[i], 0, &gc)); @@ -5042,7 +5042,7 @@ netsh_ifconfig_options(const char *type, /* delete existing DNS/WINS settings from TAP interface */ if (delete_first) { - argv_printf(&argv, "%s%sc interface ip delete %s %s all", + argv_printf(&argv, "%s%s interface ip delete %s %s all", get_win_sys_path(), NETSH_PATH_SUFFIX, type, @@ -5059,8 +5059,8 @@ netsh_ifconfig_options(const char *type, if (delete_first || !test_first || !ip_addr_member_of(addr_list[i], current)) { const char *fmt = count ? - "%s%sc interface ip add %s %s %s" - : "%s%sc interface ip set %s %s static %s"; + "%s%s interface ip add %s %s %s" + : "%s%s interface ip set %s %s static %s"; argv_printf(&argv, fmt, get_win_sys_path(), @@ -5136,7 +5136,7 @@ netsh_ifconfig(const struct tuntap_options *to, else { /* example: netsh interface ip set address my-tap static 10.3.0.1 255.255.255.0 */ - argv_printf(&argv, "%s%sc interface ip set address %s static %s %s", + argv_printf(&argv, "%s%s interface ip set address %s static %s %s", get_win_sys_path(), NETSH_PATH_SUFFIX, flex_name, @@ -5185,7 +5185,7 @@ netsh_enable_dhcp(const struct tuntap_options *to, /* example: netsh interface ip set address my-tap dhcp */ argv_printf(&argv, - "%s%sc interface ip set address %s dhcp", + "%s%s interface ip set address %s dhcp", get_win_sys_path(), NETSH_PATH_SUFFIX, actual_name); @@ -6100,7 +6100,7 @@ close_tun(struct tuntap *tt) /* netsh interface ipv6 delete address \"%s\" %s */ ifconfig_ipv6_local = print_in6_addr(tt->local_ipv6, 0, &gc); argv_printf(&argv, - "%s%sc interface ipv6 delete address %s %s store=active", + "%s%s interface ipv6 delete address %s %s store=active", get_win_sys_path(), NETSH_PATH_SUFFIX, tt->actual_name, @@ -6112,7 +6112,7 @@ close_tun(struct tuntap *tt) if (tt->options.dns6_len > 0) { argv_printf(&argv, - "%s%sc interface ipv6 delete dns %s all", + "%s%s interface ipv6 delete dns %s all", get_win_sys_path(), NETSH_PATH_SUFFIX, tt->actual_name); diff --git a/tests/unit_tests/openvpn/test_argv.c b/tests/unit_tests/openvpn/test_argv.c index 0fdd3f0a..9b67fade 100644 --- a/tests/unit_tests/openvpn/test_argv.c +++ b/tests/unit_tests/openvpn/test_argv.c @@ -9,6 +9,7 @@ #include #include #include +#include #include "argv.h" #include "buffer.h" @@ -53,23 +54,69 @@ argv_printf_cat__multiple_spaces_in_format__parsed_as_one(void **state) argv_reset(&a); } +static void +argv_printf__embedded_format_directive__replaced_in_output(void **state) +{ + struct argv a = argv_new(); + + argv_printf(&a, "", PATH1); + assert_int_equal(a.argc, 1); + assert_string_equal(a.argv[0], ""); + + argv_reset(&a); +} + +static void +argv_printf__group_sep_in_arg__fail_no_ouput(void **state) +{ + struct argv a = argv_new(); + + assert_false(argv_printf(&a, "tool --do %s", "this\035--harmful")); + assert_int_equal(a.argc, 0); + + argv_reset(&a); +} + static void argv_printf__combined_path_with_spaces__argc_correct(void **state) { struct argv a = argv_new(); - argv_printf(&a, "%s%sc", PATH1, PATH2); + argv_printf(&a, "%s%s", PATH1, PATH2); assert_int_equal(a.argc, 1); - argv_printf(&a, "%s%sc %d", PATH1, PATH2, 42); + argv_printf(&a, "%s%s %d", PATH1, PATH2, 42); assert_int_equal(a.argc, 2); - argv_printf(&a, "foo %s%sc %s x y", PATH2, PATH1, "foo"); + argv_printf(&a, "foo %s%s %s x y", PATH2, PATH1, "foo"); assert_int_equal(a.argc, 5); argv_reset(&a); } +static void +argv_printf__empty_parameter__argc_correct(void **state) +{ + struct argv a = argv_new(); + + argv_printf(&a, "%s", ""); + assert_int_equal(a.argc, 1); + + argv_printf(&a, "%s %s", PATH1, ""); + assert_int_equal(a.argc, 2); + + argv_printf(&a, "%s %s %s", PATH1, "", PARAM1); + assert_int_equal(a.argc, 3); + + argv_printf(&a, "%s %s %s %s", PATH1, "", "", PARAM1); + assert_int_equal(a.argc, 4); + + argv_printf(&a, "%s %s", "", PARAM1); + assert_int_equal(a.argc, 2); + + argv_free(&a); +} + static void argv_parse_cmd__command_string__argc_correct(void **state) { @@ -113,7 +160,7 @@ argv_str__multiple_argv__correct_output(void **state) struct gc_arena gc = gc_new(); const char *output; - argv_printf(&a, "%s%sc", PATH1, PATH2); + argv_printf(&a, "%s%s", PATH1, PATH2); argv_printf_cat(&a, "%s", PARAM1); argv_printf_cat(&a, "%s", PARAM2); argv_printf_cat(&a, "%d", -1); @@ -172,7 +219,10 @@ main(void) const struct CMUnitTest tests[] = { cmocka_unit_test(argv_printf__multiple_spaces_in_format__parsed_as_one), cmocka_unit_test(argv_printf_cat__multiple_spaces_in_format__parsed_as_one), + cmocka_unit_test(argv_printf__embedded_format_directive__replaced_in_output), + 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_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), From patchwork Fri Oct 19 04:56:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Sommerseth X-Patchwork-Id: 563 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director11.mail.ord1d.rsapps.net ([172.27.255.51]) by backend30.mail.ord1d.rsapps.net with LMTP id iEwtKwH/yVvhWAAAIUCqbw for ; Fri, 19 Oct 2018 11:57:53 -0400 Received: from proxy1.mail.iad3a.rsapps.net ([172.27.255.51]) by director11.mail.ord1d.rsapps.net with LMTP id iISJKAH/yVuRKAAAvGGmqA ; Fri, 19 Oct 2018 11:57:53 -0400 Received: from smtp7.gate.iad3a ([172.27.255.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy1.mail.iad3a.rsapps.net with LMTP id KA2FIgH/yVswRwAA8TVjwQ ; Fri, 19 Oct 2018 11:57:53 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp7.gate.iad3a.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=openvpn.net X-Suspicious-Flag: YES X-Classification-ID: bc30e0f8-d3b7-11e8-b322-525400bbebb8-1-1 Received: from [216.105.38.7] ([216.105.38.7:40054] helo=lists.sourceforge.net) by smtp7.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 3E/D6-08708-00FF9CB5; Fri, 19 Oct 2018 11:57:53 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1gDX9K-00064B-49; Fri, 19 Oct 2018 15:57:18 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1gDX9I-00063x-Vc for openvpn-devel@lists.sourceforge.net; Fri, 19 Oct 2018 15:57:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:Cc: To:From:Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=fgxEkhVYiQiIwkKFIb2oD+sO08UMBhQve3qIcS7Gr74=; b=dn3X6sF9+U268ErX1PWUz/gXpP 1bwV8Q3oDaAeHP1hZFO2VkVg+md9WX6Jl82smbk3wwKdXZJKOYHQZWEwWCSSR+eZOo9FVCvvJ0pmR s4L4mM8qLL5WycsfKraE9rn56+pHG3bYuSxi4/cNErWxRVon4AMPRjuf4LGqLP3hfhD8=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To :MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=fgxEkhVYiQiIwkKFIb2oD+sO08UMBhQve3qIcS7Gr74=; b=dk7gy0Y0b3AYpg1qIKSxOoPsHI fYM0imXgOHJ27dXFgsZCamBVSdHUYjqBVFcfo4HSimiWAI64zKUjB7yBPKaxnTUFBbsLKB1zq4av2 CPUHVLzvQO3FLK8dUKbvIA5os2W7PsAGmzwBYrF4oV56z6cOJqMj5l9DaYuzwiWOGe8w=; Received: from mx0.basenordic.cloud ([185.212.44.139]) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) id 1gDX9G-0057CY-SH for openvpn-devel@lists.sourceforge.net; Fri, 19 Oct 2018 15:57:16 +0000 Received: from localhost (unknown [IPv6:::1]) by mx0.basenordic.cloud (Postfix) with ESMTP id 756FA849D5E; Fri, 19 Oct 2018 15:57:08 +0000 (UTC) Received: from mx0.basenordic.cloud ([127.0.0.1]) by localhost (winterfell.topphemmelig.net [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yWVbf4ZqsQ13; Fri, 19 Oct 2018 17:57:06 +0200 (CEST) Received: from zimbra.sommerseth.email (zimbra.sommerseth.email [172.16.33.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx0.basenordic.cloud (Postfix) with ESMTPS id 7F477849D4C; Fri, 19 Oct 2018 17:56:51 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by zimbra.sommerseth.email (Postfix) with ESMTP id 15F1E51E21D9; Fri, 19 Oct 2018 17:56:51 +0200 (CEST) Received: from zimbra.sommerseth.email ([127.0.0.1]) by localhost (zimbra.sommerseth.email [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id HYrkRkroPmio; Fri, 19 Oct 2018 17:56:50 +0200 (CEST) Received: from optimus.homebase.sommerseths.net (unknown [10.35.7.3]) by zimbra.sommerseth.email (Postfix) with ESMTPS id CBC4751E21D8; Fri, 19 Oct 2018 17:56:50 +0200 (CEST) From: David Sommerseth To: openvpn-devel@lists.sourceforge.net Date: Fri, 19 Oct 2018 17:56:36 +0200 Message-Id: <20181019155638.9498-3-davids@openvpn.net> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181019155638.9498-1-davids@openvpn.net> References: <20181019155638.9498-1-davids@openvpn.net> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 SPF_HELO_PASS SPF: HELO matches SPF record 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different -0.0 SPF_PASS SPF: sender matches SPF record 0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1gDX9G-0057CY-SH Subject: [Openvpn-devel] [PATCH 2/4] argv: do fewer memory re-allocations X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox From: Heiko Hund 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 the argv_reset functionality so that it can be used to actually reset the argv without re-allocation. Signed-off-by: Heiko Hund --- src/openvpn/argv.c | 81 ++++++++++++++-------------- src/openvpn/argv.h | 2 +- src/openvpn/console_systemd.c | 2 +- src/openvpn/init.c | 15 ++---- src/openvpn/lladdr.c | 2 +- src/openvpn/multi.c | 10 ++-- src/openvpn/options.c | 2 +- src/openvpn/plugin.c | 2 +- src/openvpn/route.c | 8 +-- src/openvpn/socket.c | 4 +- src/openvpn/ssl_verify.c | 6 +-- src/openvpn/tun.c | 38 ++++++------- tests/unit_tests/openvpn/test_argv.c | 41 +++++++++----- 13 files changed, 110 insertions(+), 103 deletions(-) diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c index 9606f382..ff6d0b46 100644 --- a/src/openvpn/argv.c +++ b/src/openvpn/argv.c @@ -40,34 +40,6 @@ #include "env_set.h" #include "options.h" -static void -argv_init(struct argv *a) -{ - a->capacity = 0; - a->argc = 0; - a->argv = NULL; -} - -struct argv -argv_new(void) -{ - struct argv ret; - argv_init(&ret); - return ret; -} - -void -argv_reset(struct argv *a) -{ - size_t i; - for (i = 0; i < a->argc; ++i) - { - free(a->argv[i]); - } - free(a->argv); - argv_init(a); -} - static void argv_extend(struct argv *a, const size_t newcap) { @@ -86,6 +58,46 @@ argv_extend(struct argv *a, const size_t newcap) } } +static void +argv_init(struct argv *a) +{ + a->capacity = 0; + a->argc = 0; + a->argv = NULL; + argv_extend(a, 8); +} + +struct argv +argv_new(void) +{ + struct argv ret; + argv_init(&ret); + return ret; +} + +void +argv_free(struct argv *a) +{ + size_t i; + for (i = 0; i < a->argc; ++i) + { + free(a->argv[i]); + } + free(a->argv); +} + +static void +argv_reset(struct argv *a) +{ + size_t i; + for (i = 0; i < a->argc; ++i) + { + free(a->argv[i]); + a->argv[i] = NULL; + } + a->argc = 0; +} + static void argv_grow(struct argv *a, const size_t add) { @@ -134,14 +146,7 @@ argv_insert_head(const struct argv *a, const char *head) const char * argv_str(const struct argv *a, struct gc_arena *gc, const unsigned int flags) { - if (a->argv) - { - return print_argv((const char **)a->argv, gc, flags); - } - else - { - return ""; - } + return print_argv((const char **)a->argv, gc, flags); } void @@ -229,8 +234,6 @@ argv_printf_arglist(struct argv *a, const char *format, va_list arglist) va_list tmplist; int len; - argv_extend(a, 1); /* ensure trailing NULL */ - argc = a->argc; f = argv_prep_format(format, delim, &argc, &gc); if (f == NULL) @@ -270,7 +273,6 @@ argv_printf_arglist(struct argv *a, const char *format, va_list arglist) { /* Someone snuck in a GS (0x1D), fail gracefully */ argv_reset(a); - argv_extend(a, 1); /* ensure trailing NULL */ goto out; } @@ -315,7 +317,6 @@ argv_parse_cmd(struct argv *a, const char *s) struct gc_arena gc = gc_new(); argv_reset(a); - argv_extend(a, 1); /* ensure trailing NULL */ nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0, D_ARGV_PARSE_CMD, &gc); if (nparms) diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h index b9105a43..989cd297 100644 --- a/src/openvpn/argv.h +++ b/src/openvpn/argv.h @@ -40,7 +40,7 @@ struct argv { struct argv argv_new(void); -void argv_reset(struct argv *a); +void argv_free(struct argv *a); const char *argv_str(const struct argv *a, struct gc_arena *gc, const unsigned int flags); diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c index 8d9e825b..c7cf1ada 100644 --- a/src/openvpn/console_systemd.c +++ b/src/openvpn/console_systemd.c @@ -85,7 +85,7 @@ get_console_input_systemd(const char *prompt, const bool echo, char *input, cons } close(std_out); - argv_reset(&argv); + argv_free(&argv); return ret; } diff --git a/src/openvpn/init.c b/src/openvpn/init.c index e5e6e85f..7dab4f21 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -163,7 +163,7 @@ run_up_down(const char *command, msg(M_FATAL, "ERROR: up/down plugin call failed"); } - argv_reset(&argv); + argv_free(&argv); } if (command) @@ -176,7 +176,7 @@ run_up_down(const char *command, ifconfig_local, ifconfig_remote, context); argv_msg(M_INFO, &argv); openvpn_run_script(&argv, es, S_FATAL, "--up/--down"); - argv_reset(&argv); + argv_free(&argv); } gc_free(&gc); @@ -844,15 +844,6 @@ init_static(void) return false; #endif -#ifdef ARGV_TEST - { - void argv_test(void); - - argv_test(); - return false; - } -#endif - #ifdef PRNG_TEST { struct gc_arena gc = gc_new(); @@ -1639,7 +1630,7 @@ do_route(const struct options *options, setenv_str(es, "script_type", "route-up"); argv_parse_cmd(&argv, options->route_script); openvpn_run_script(&argv, es, 0, "--route-up"); - argv_reset(&argv); + argv_free(&argv); } #ifdef _WIN32 diff --git a/src/openvpn/lladdr.c b/src/openvpn/lladdr.c index e6b26fc9..22857eb7 100644 --- a/src/openvpn/lladdr.c +++ b/src/openvpn/lladdr.c @@ -69,6 +69,6 @@ set_lladdr(const char *ifname, const char *lladdr, msg(M_INFO, "TUN/TAP link layer address set to %s", lladdr); } - argv_reset(&argv); + argv_free(&argv); return r; } diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 8440f311..c8c04f00 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -138,7 +138,7 @@ learn_address_script(const struct multi_context *m, msg(M_WARN, "WARNING: learn-address plugin call failed"); ret = false; } - argv_reset(&argv); + argv_free(&argv); } if (m->top.options.learn_address_script) @@ -155,7 +155,7 @@ learn_address_script(const struct multi_context *m, { ret = false; } - argv_reset(&argv); + argv_free(&argv); } gc_free(&gc); @@ -593,7 +593,7 @@ multi_client_disconnect_script(struct multi_context *m, setenv_str(mi->context.c2.es, "script_type", "client-disconnect"); argv_parse_cmd(&argv, mi->context.options.client_disconnect_script); openvpn_run_script(&argv, mi->context.c2.es, 0, "--client-disconnect"); - argv_reset(&argv); + argv_free(&argv); } #ifdef MANAGEMENT_DEF_AUTH if (management) @@ -1908,7 +1908,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) } script_depr_failed: - argv_reset(&argv); + argv_free(&argv); } /* V2 callback, use a plugin_return struct for passing back return info */ @@ -1971,7 +1971,7 @@ script_depr_failed: } script_failed: - argv_reset(&argv); + argv_free(&argv); } /* diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 5c669e91..e4aa9a11 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3228,7 +3228,7 @@ check_cmd_access(const char *command, const char *opt, const char *chroot) return_code = true; } - argv_reset(&argv); + argv_free(&argv); return return_code; } diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c index 4d17c821..09424c61 100644 --- a/src/openvpn/plugin.c +++ b/src/openvpn/plugin.c @@ -587,7 +587,7 @@ plugin_call_item(const struct plugin *p, p->so_pathname); } - argv_reset(&a); + argv_free(&a); gc_free(&gc); } return status; diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 9649d197..cbee7a20 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -1815,7 +1815,7 @@ done: { r->flags &= ~RT_ADDED; } - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); } @@ -2122,7 +2122,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag { r6->flags &= ~RT_ADDED; } - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); } @@ -2314,7 +2314,7 @@ delete_route(struct route_ipv4 *r, done: r->flags &= ~RT_ADDED; - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); } @@ -2548,7 +2548,7 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, unsigned msg(M_FATAL, "Sorry, but I don't know how to do 'route ipv6' commands on this operating system. Try putting your routes in a --route-down script"); #endif /* if defined(TARGET_LINUX) */ - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); } diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 3f2b97e4..694e6f70 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -2467,7 +2467,7 @@ link_socket_connection_initiated(const struct buffer *buf, { msg(M_WARN, "WARNING: ipchange plugin call failed"); } - argv_reset(&argv); + argv_free(&argv); } /* Process --ipchange option */ @@ -2477,7 +2477,7 @@ link_socket_connection_initiated(const struct buffer *buf, setenv_str(es, "script_type", "ipchange"); ipchange_fmt(true, &argv, info, &gc); openvpn_run_script(&argv, es, 0, "--ipchange"); - argv_reset(&argv); + argv_free(&argv); } gc_free(&gc); diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 61872251..ede6da55 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -522,7 +522,7 @@ verify_cert_call_plugin(const struct plugin_list *plugins, struct env_set *es, ret = plugin_call_ssl(plugins, OPENVPN_PLUGIN_TLS_VERIFY, &argv, NULL, es, cert_depth, cert); - argv_reset(&argv); + argv_free(&argv); if (ret == OPENVPN_PLUGIN_FUNC_SUCCESS) { @@ -615,7 +615,7 @@ verify_cert_call_command(const char *verify_command, struct env_set *es, cleanup: gc_free(&gc); - argv_reset(&argv); + argv_free(&argv); if (ret) { @@ -1159,7 +1159,7 @@ done: platform_unlink(tmp_file); } - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); return ret; } diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index f80aedf8..94087bd2 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -1012,7 +1012,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu, #endif /* outer "if defined(TARGET_xxx)" conditional */ gc_free(&gc); - argv_reset(&argv); + argv_free(&argv); } /** @@ -1400,7 +1400,7 @@ do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu, #endif /* if defined(TARGET_LINUX) */ gc_free(&gc); - argv_reset(&argv); + argv_free(&argv); } /* execute the ifconfig command through the shell */ @@ -1968,7 +1968,7 @@ undo_ifconfig_ipv4(struct tuntap *tt, struct gc_arena *gc) argv_msg(M_INFO, &argv); openvpn_execve_check(&argv, NULL, 0, "Linux ip addr del failed"); - argv_reset(&argv); + argv_free(&argv); } void @@ -1988,7 +1988,7 @@ undo_ifconfig_ipv6(struct tuntap *tt, struct gc_arena *gc) argv_msg(M_INFO, &argv); openvpn_execve_check(&argv, NULL, 0, "Linux ip -6 addr del failed"); - argv_reset(&argv); + argv_free(&argv); } void @@ -2280,7 +2280,7 @@ solaris_close_tun(struct tuntap *tt) IFCONFIG_PATH, tt->actual_name ); argv_msg(M_INFO, &argv); openvpn_execve_check(&argv, NULL, 0, "Solaris ifconfig inet6 unplumb failed"); - argv_reset(&argv); + argv_free(&argv); } if (tt->ip_fd >= 0) @@ -2365,7 +2365,7 @@ solaris_error_close(struct tuntap *tt, const struct env_set *es, openvpn_execve_check(&argv, es, 0, "Solaris ifconfig unplumb failed"); close_tun(tt); msg(M_FATAL, "Solaris ifconfig failed"); - argv_reset(&argv); + argv_free(&argv); } int @@ -2453,7 +2453,7 @@ close_tun(struct tuntap *tt) openvpn_execve_check(&argv, NULL, 0, "OpenBSD 'destroy tun interface' failed (non-critical)"); free(tt); - argv_reset(&argv); + argv_free(&argv); } int @@ -2539,7 +2539,7 @@ close_tun(struct tuntap *tt) openvpn_execve_check(&argv, NULL, 0, "NetBSD 'destroy tun interface' failed (non-critical)"); free(tt); - argv_reset(&argv); + argv_free(&argv); } static inline int @@ -2680,7 +2680,7 @@ close_tun(struct tuntap *tt) "FreeBSD 'destroy tun interface' failed (non-critical)"); free(tt); - argv_reset(&argv); + argv_free(&argv); } int @@ -3045,7 +3045,7 @@ close_tun(struct tuntap *tt) close_tun_generic(tt); free(tt); - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); } @@ -3149,7 +3149,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun env_set_add( es, "ODMDIR=/etc/objrepos" ); openvpn_execve_check(&argv, es, S_FATAL, "AIX 'create tun interface' failed"); env_set_destroy(es); - argv_reset(&argv); + argv_free(&argv); } else { @@ -3200,7 +3200,7 @@ close_tun(struct tuntap *tt) free(tt); env_set_destroy(es); - argv_reset(&argv); + argv_free(&argv); } int @@ -4878,14 +4878,14 @@ ipconfig_register_dns(const struct env_set *es) WIN_IPCONFIG_PATH_SUFFIX); argv_msg(D_TUNTAP_INFO, &argv); status = openvpn_execve_check(&argv, es, 0, err); - argv_reset(&argv); + argv_free(&argv); argv_printf(&argv, "%s%s /registerdns", get_win_sys_path(), WIN_IPCONFIG_PATH_SUFFIX); argv_msg(D_TUNTAP_INFO, &argv); status = openvpn_execve_check(&argv, es, 0, err); - argv_reset(&argv); + argv_free(&argv); netcmd_semaphore_release(); msg(D_TUNTAP_INFO, "End ipconfig commands for register-dns..."); @@ -5010,7 +5010,7 @@ netsh_set_dns6_servers(const struct in6_addr *addr_list, netsh_command(&argv, 1, (i==0) ? M_FATAL : M_NONFATAL); } - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); } @@ -5082,7 +5082,7 @@ netsh_ifconfig_options(const char *type, } } - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); } @@ -5173,7 +5173,7 @@ netsh_ifconfig(const struct tuntap_options *to, BOOL_CAST(flags & NI_TEST_FIRST)); } - argv_reset(&argv); + argv_free(&argv); gc_free(&gc); } @@ -5192,7 +5192,7 @@ netsh_enable_dhcp(const struct tuntap_options *to, netsh_command(&argv, 4, M_FATAL); - argv_reset(&argv); + argv_free(&argv); } /* Enable dhcp on tap adapter using iservice */ @@ -6118,7 +6118,7 @@ close_tun(struct tuntap *tt) tt->actual_name); netsh_command(&argv, 1, M_WARN); } - argv_reset(&argv); + argv_free(&argv); } } #if 1 diff --git a/tests/unit_tests/openvpn/test_argv.c b/tests/unit_tests/openvpn/test_argv.c index 9b67fade..fde0ba45 100644 --- a/tests/unit_tests/openvpn/test_argv.c +++ b/tests/unit_tests/openvpn/test_argv.c @@ -39,7 +39,7 @@ argv_printf__multiple_spaces_in_format__parsed_as_one(void **state) argv_printf(&a, " %s %s %d ", PATH1, PATH2, 42); assert_int_equal(a.argc, 3); - argv_reset(&a); + argv_free(&a); } static void @@ -51,7 +51,7 @@ argv_printf_cat__multiple_spaces_in_format__parsed_as_one(void **state) argv_printf_cat(&a, " %s %s", PATH2, PARAM1); assert_int_equal(a.argc, 3); - argv_reset(&a); + argv_free(&a); } static void @@ -63,7 +63,7 @@ argv_printf__embedded_format_directive__replaced_in_output(void **state) assert_int_equal(a.argc, 1); assert_string_equal(a.argv[0], ""); - argv_reset(&a); + argv_free(&a); } static void @@ -74,7 +74,7 @@ argv_printf__group_sep_in_arg__fail_no_ouput(void **state) assert_false(argv_printf(&a, "tool --do %s", "this\035--harmful")); assert_int_equal(a.argc, 0); - argv_reset(&a); + argv_free(&a); } static void @@ -91,7 +91,7 @@ argv_printf__combined_path_with_spaces__argc_correct(void **state) argv_printf(&a, "foo %s%s %s x y", PATH2, PATH1, "foo"); assert_int_equal(a.argc, 5); - argv_reset(&a); + argv_free(&a); } static void @@ -125,7 +125,7 @@ argv_parse_cmd__command_string__argc_correct(void **state) argv_parse_cmd(&a, SCRIPT_CMD); assert_int_equal(a.argc, 3); - argv_reset(&a); + argv_free(&a); } static void @@ -137,7 +137,7 @@ argv_parse_cmd__command_and_extra_options__argc_correct(void **state) argv_printf_cat(&a, "bar baz %d %s", 42, PATH1); assert_int_equal(a.argc, 7); - argv_reset(&a); + argv_free(&a); } static void @@ -150,7 +150,21 @@ argv_printf_cat__used_twice__argc_correct(void **state) argv_printf_cat(&a, "foo"); assert_int_equal(a.argc, 5); - argv_reset(&a); + argv_free(&a); +} + +static void +argv_str__empty_argv__empty_output(void **state) +{ + struct argv a = argv_new(); + struct gc_arena gc = gc_new(); + const char *output; + + output = argv_str(&a, &gc, PA_BRACKET); + assert_string_equal(output, ""); + + argv_free(&a); + gc_free(&gc); } static void @@ -170,7 +184,7 @@ argv_str__multiple_argv__correct_output(void **state) assert_string_equal(output, "[" PATH1 PATH2 "] [" PARAM1 "] [" PARAM2 "]" " [-1] [4294967295] [1]"); - argv_reset(&a); + argv_free(&a); gc_free(&gc); } @@ -183,9 +197,9 @@ argv_insert_head__empty_argv__head_only(void **state) b = argv_insert_head(&a, PATH1); assert_int_equal(b.argc, 1); assert_string_equal(b.argv[0], PATH1); - argv_reset(&b); + argv_free(&b); - argv_reset(&a); + argv_free(&a); } static void @@ -208,9 +222,9 @@ argv_insert_head__non_empty_argv__head_added(void **state) assert_string_equal(b.argv[i], a.argv[i - 1]); } } - argv_reset(&b); + argv_free(&b); - argv_reset(&a); + argv_free(&a); } int @@ -226,6 +240,7 @@ main(void) 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), + cmocka_unit_test(argv_str__empty_argv__empty_output), cmocka_unit_test(argv_str__multiple_argv__correct_output), cmocka_unit_test(argv_insert_head__non_empty_argv__head_added), }; From patchwork Fri Oct 19 04:56:37 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Sommerseth X-Patchwork-Id: 560 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director12.mail.ord1d.rsapps.net ([172.27.255.9]) by backend30.mail.ord1d.rsapps.net with LMTP id ILhpMfn+yVvGMQAAIUCqbw for ; Fri, 19 Oct 2018 11:57:45 -0400 Received: from proxy11.mail.iad3a.rsapps.net ([172.27.255.9]) by director12.mail.ord1d.rsapps.net with LMTP id KAT6Lvn+yVsXZgAAIasKDg ; Fri, 19 Oct 2018 11:57:45 -0400 Received: from smtp26.gate.iad3a ([172.27.255.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy11.mail.iad3a.rsapps.net with LMTP id KJOYKfn+yVuvZwAAxCvdqw ; Fri, 19 Oct 2018 11:57:45 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp26.gate.iad3a.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=openvpn.net X-Suspicious-Flag: YES X-Classification-ID: b7280f50-d3b7-11e8-86eb-52540063aac2-1-1 Received: from [216.105.38.7] ([216.105.38.7:51840] helo=lists.sourceforge.net) by smtp26.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 0F/D4-27063-8FEF9CB5; Fri, 19 Oct 2018 11:57:45 -0400 Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1gDX98-0006I9-S0; Fri, 19 Oct 2018 15:57:06 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1gDX97-0006I2-Ft for openvpn-devel@lists.sourceforge.net; Fri, 19 Oct 2018 15:57:05 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:Cc: To:From:Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=3AkCssfwRBayIgFcR80q6a4S/06loSh9umPIfPBs924=; b=JXZlzmNYx7oq2AXJytO3wPQRSc voloPV3D2IDnCKkJUyQJSAxsLCmVSsOcGzi8fda+7flDPJkygGQMd4uCObtv9Wa8GKpTWHcIziSIT rtcCxRepEQFu3cHSpNXJd8lYQI/jVph5F8mU/6CJ162s//kLARSjeWtLTE4aAAsoDBq0=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To :MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=3AkCssfwRBayIgFcR80q6a4S/06loSh9umPIfPBs924=; b=kzW7wwNFm1HW2izJbCH+em4Juq VuDP+LK+liztTZBsLnjFt+xHmquVTWEXzI38uH8b8t1pZORdRnHbpOHCVYREysaLyQTeCJmMigb8m B6BQMyVXQVLKFKgxWiXmcqJtjfmeYA8115gpohb42WhBphpYY0srJdp528ghjD7HeLTA=; Received: from mx0.basenordic.cloud ([185.212.44.139]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) id 1gDX95-0082rc-Ns for openvpn-devel@lists.sourceforge.net; Fri, 19 Oct 2018 15:57:05 +0000 Received: from localhost (unknown [IPv6:::1]) by mx0.basenordic.cloud (Postfix) with ESMTP id 39A5B849D5E; Fri, 19 Oct 2018 15:56:57 +0000 (UTC) Received: from mx0.basenordic.cloud ([127.0.0.1]) by localhost (winterfell.topphemmelig.net [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UsS3kM5hDR7F; Fri, 19 Oct 2018 17:56:56 +0200 (CEST) Received: from zimbra.sommerseth.email (zimbra.sommerseth.email [172.16.33.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx0.basenordic.cloud (Postfix) with ESMTPS id E91C681E901; Fri, 19 Oct 2018 17:56:52 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by zimbra.sommerseth.email (Postfix) with ESMTP id 7378D51E21D9; Fri, 19 Oct 2018 17:56:52 +0200 (CEST) Received: from zimbra.sommerseth.email ([127.0.0.1]) by localhost (zimbra.sommerseth.email [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id lnbOF2FvBEYx; Fri, 19 Oct 2018 17:56:52 +0200 (CEST) Received: from optimus.homebase.sommerseths.net (unknown [10.35.7.3]) by zimbra.sommerseth.email (Postfix) with ESMTPS id 3115B51E21D8; Fri, 19 Oct 2018 17:56:52 +0200 (CEST) From: David Sommerseth To: openvpn-devel@lists.sourceforge.net Date: Fri, 19 Oct 2018 17:56:37 +0200 Message-Id: <20181019155638.9498-4-davids@openvpn.net> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181019155638.9498-1-davids@openvpn.net> References: <20181019155638.9498-1-davids@openvpn.net> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 SPF_HELO_PASS SPF: HELO matches SPF record 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different -0.0 SPF_PASS SPF: sender matches SPF record 0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1gDX95-0082rc-Ns Subject: [Openvpn-devel] [PATCH 3/4] Add gc_arena to struct argv to save allocations X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox 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, &a->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(&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 @@ -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(&r, string_alloc(a->argv[i], NULL)); + argv_append(&r, string_alloc(a->argv[i], &r.gc)); } } 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, &r.gc); 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, &gc); + buf = gc_malloc(size, false, &a->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, &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 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(&a); } +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) { @@ -237,6 +259,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), From patchwork Fri Oct 19 04:56:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Sommerseth X-Patchwork-Id: 562 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director10.mail.ord1d.rsapps.net ([172.27.255.51]) by backend30.mail.ord1d.rsapps.net with LMTP id 6OSJF//+yVsmOQAAIUCqbw for ; Fri, 19 Oct 2018 11:57:51 -0400 Received: from proxy21.mail.iad3a.rsapps.net ([172.27.255.51]) by director10.mail.ord1d.rsapps.net with LMTP id yN96FP/+yVtTYwAApN4f7A ; Fri, 19 Oct 2018 11:57:51 -0400 Received: from smtp34.gate.iad3a ([172.27.255.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy21.mail.iad3a.rsapps.net with LMTP id +LOqDf/+yVuOAQAASBQwCQ ; Fri, 19 Oct 2018 11:57:51 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp34.gate.iad3a.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=openvpn.net X-Suspicious-Flag: YES X-Classification-ID: bae27a54-d3b7-11e8-8207-525400865cc7-1-1 Received: from [216.105.38.7] ([216.105.38.7:33807] helo=lists.sourceforge.net) by smtp34.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 94/72-21168-EFEF9CB5; Fri, 19 Oct 2018 11:57:51 -0400 Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1gDX9C-0006IX-UN; Fri, 19 Oct 2018 15:57:10 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1gDX9B-0006IO-PT for openvpn-devel@lists.sourceforge.net; Fri, 19 Oct 2018 15:57:09 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:To: From:Sender:Reply-To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=TSAKTXjq2/T/lR+rBW4INKUEeM/IR20YNRn7CsO/iWY=; b=QVhaKKqHB66HB9PAdUiOr3iV3+ 6OGaWqeXYy4T0AJjS05T/IriVy/8w6Pu/oWrvxz+HkryH7j1hZVysQE3XlQAjRPRa+L0oREY50jJf PAgBxywIwdIYIb83PziStygew7vSIXrdVL5l6oolceGCrRi236IqAHr7/p4cK/8Rma0M=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc :MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=TSAKTXjq2/T/lR+rBW4INKUEeM/IR20YNRn7CsO/iWY=; b=KNnNqeMtkzj+c2E3fbF584SuTS IeDjegJ2cEQEukxVux4KtXOzAQGLMPmzttKc341emt+JEO6G4BS6I4wowaZqHQXUHgGNovk/jeEhf iSkmr29OSVFbkE/y0Lp3a9Ct/yijkNwTu8+kFWqgs3wB8jLcxQ+9KjT9q3ADagrfCsyg=; Received: from mx0.basenordic.cloud ([185.212.44.139]) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) id 1gDX99-0057Bv-QR for openvpn-devel@lists.sourceforge.net; Fri, 19 Oct 2018 15:57:09 +0000 Received: from localhost (unknown [IPv6:::1]) by mx0.basenordic.cloud (Postfix) with ESMTP id 956E981E901 for ; Fri, 19 Oct 2018 15:56:58 +0000 (UTC) Received: from mx0.basenordic.cloud ([127.0.0.1]) by localhost (winterfell.topphemmelig.net [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id TBEcLhkFBEFM for ; Fri, 19 Oct 2018 17:56:57 +0200 (CEST) Received: from zimbra.sommerseth.email (zimbra.sommerseth.email [172.16.33.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx0.basenordic.cloud (Postfix) with ESMTPS id 9B43F8251CB for ; Fri, 19 Oct 2018 17:56:54 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by zimbra.sommerseth.email (Postfix) with ESMTP id 2AB6240B371E for ; Fri, 19 Oct 2018 17:56:54 +0200 (CEST) Received: from zimbra.sommerseth.email ([127.0.0.1]) by localhost (zimbra.sommerseth.email [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id P4ZjdrT-WiWA for ; Fri, 19 Oct 2018 17:56:54 +0200 (CEST) Received: from optimus.homebase.sommerseths.net (unknown [10.35.7.3]) by zimbra.sommerseth.email (Postfix) with ESMTPS id EAF8051E21D8 for ; Fri, 19 Oct 2018 17:56:53 +0200 (CEST) From: David Sommerseth To: openvpn-devel@lists.sourceforge.net Date: Fri, 19 Oct 2018 17:56:38 +0200 Message-Id: <20181019155638.9498-5-davids@openvpn.net> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181019155638.9498-1-davids@openvpn.net> References: <20181019155638.9498-1-davids@openvpn.net> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 SPF_HELO_PASS SPF: HELO matches SPF record 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different -0.0 SPF_PASS SPF: sender matches SPF record 0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1gDX99-0057Bv-QR Subject: [Openvpn-devel] [PATCH 4/4] Documented all the argv related code with minor refactoring X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox Added doxygen comments for all the functions in argv.c. There are some slight refactoring, renaming a few variables to make their use case more obvious and ensure lines do not break our 80-chars per line coding style limit. Signed-off-by: David Sommerseth --- src/openvpn/argv.c | 246 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 209 insertions(+), 37 deletions(-) diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c index 39c6fa9e..d3b47d97 100644 --- a/src/openvpn/argv.c +++ b/src/openvpn/argv.c @@ -40,6 +40,13 @@ #include "env_set.h" #include "options.h" +/** + * Resizes the list of arguments struct argv can carry. This resize + * operation will only increase the size, never decrease the size. + * + * @param *a Valid pointer to a struct argv to resize + * @param newcap size_t with the new size of the argument list. + */ static void argv_extend(struct argv *a, const size_t newcap) { @@ -57,6 +64,12 @@ argv_extend(struct argv *a, const size_t newcap) } } +/** + * Initialise an already allocated struct argv. + * It is expected that the input argument is a valid pointer. + * + * @param *a Pointer to a struct argv to initialise + */ static void argv_init(struct argv *a) { @@ -67,6 +80,12 @@ argv_init(struct argv *a) argv_extend(a, 8); } +/** + * Allocates a new struct argv and ensures it is initialised. + * Note that it does not return a pointer, but a struct argv directly. + * + * @returns Returns an initialised and empty struct argv. + */ struct argv argv_new(void) { @@ -75,12 +94,24 @@ argv_new(void) return ret; } +/** + * Frees all memory allocations allocated by the struct argv + * related functions. + * + * @param *a Valid pointer to a struct argv to release memory from + */ void argv_free(struct argv *a) { gc_free(&a->gc); } +/** + * Resets the struct argv to an initial state. No memory buffers + * will be released by this call. + * + * @param *a Valid pointer to a struct argv to resize + */ static void argv_reset(struct argv *a) { @@ -95,6 +126,19 @@ argv_reset(struct argv *a) } } +/** + * Extends an existing struct argv to carry minimum 'add' number + * of new arguments. This builds on argv_extend(), which ensures the + * new size will only be higher than the current capacity. + * + * The new size is also calculated based on the result of adjust_power_of_2(). + * This approach ensures that the list does grow bulks and only when the + * current limit is reached. + * + * @param *a Valid pointer to the struct argv to extend + * @param add size_t with the number of elements to add. + * + */ static void argv_grow(struct argv *a, const size_t add) { @@ -103,15 +147,39 @@ argv_grow(struct argv *a, const size_t add) argv_extend(a, adjust_power_of_2(newargc)); } +/** + * Appends a string to to the list of arguments stored in a struct argv + * This will ensure the list size in struct argv has the needed capacity to + * store the value. + * + * @param *a struct argv where to append the new string value + * @param *str Pointer to string to append. The provided string *MUST* have + * been malloc()ed or NULL. + */ static void -argv_append(struct argv *a, char *str) /* str must have been gc_malloced or be NULL */ +argv_append(struct argv *a, char *str) { argv_grow(a, 1); a->argv[a->argc++] = str; } +/** + * Clones a struct argv with all the contents to a new allocated struct argv. + * If 'headroom' is larger than 0, it will create a head-room in front of the + * values being copied from the source input. + * + * + * @param *source Valid pointer to the source struct argv to clone. It may + * be NULL. + * @param headroom Number of slots to leave empty in front of the slots + * copied from the source. + * + * @returns Returns a new struct argv containing a copy of the source + * struct argv, with the given headroom in front of the copy. + * + */ static struct argv -argv_clone(const struct argv *a, const size_t headroom) +argv_clone(const struct argv *source, const size_t headroom) { struct argv r; size_t i; @@ -121,16 +189,24 @@ argv_clone(const struct argv *a, const size_t headroom) { argv_append(&r, NULL); } - if (a) + if (source) { - for (i = 0; i < a->argc; ++i) + for (i = 0; i < source->argc; ++i) { - argv_append(&r, string_alloc(a->argv[i], &r.gc)); + argv_append(&r, string_alloc(source->argv[i], &r.gc)); } } return r; } +/** + * Inserts an argument string in front of all other argument slots. + * + * @param *a Valid pointer to the struct argv to insert the argument into + * @param *head Pointer to the char * string with the argument to insert + * + * @returns Returns a new struct argv with the inserted argument in front + */ struct argv argv_insert_head(const struct argv *a, const char *head) { @@ -140,12 +216,32 @@ argv_insert_head(const struct argv *a, const char *head) return r; } +/** + * Generate a single string with all the arguments in a struct argv + * concatenated. + * + * @param *a Valid pointer to the struct argv with the arguments to list + * @param *gc Pointer to a struct gc_arena managed buffer + * @param flags Flags passed to the print_argv() function. + * + * @returns Returns a string generated by print_argv() with all the arguments + * concatenated. If the argument count is 0, it will return an empty + * string. The return string is allocated in the gc_arena managed + * buffer. If the gc_arena pointer is NULL, the returned string + * must be free()d explicitly to avoid memory leaks. + */ const char * argv_str(const struct argv *a, struct gc_arena *gc, const unsigned int flags) { return print_argv((const char **)a->argv, gc, flags); } +/** + * Write the arguments stored in a struct argv via the msg() command. + * + * @param msglev Integer with the message level used by msg(). + * @param *a Valid pointer to the struct argv with the arguments to write. + */ void argv_msg(const int msglev, const struct argv *a) { @@ -154,6 +250,15 @@ argv_msg(const int msglev, const struct argv *a) gc_free(&gc); } +/** + * Similar to argv_msg() but prefixes the messages being written with a + * given string. + * + * @param msglev Integer with the message level used by msg(). + * @param *a Valid pointer to the struct argv with the arguments to write + * @param *prefix Valid char * pointer to the prefix string + * + */ void argv_msg_prefix(const int msglev, const struct argv *a, const char *prefix) { @@ -162,16 +267,29 @@ argv_msg_prefix(const int msglev, const struct argv *a, const char *prefix) gc_free(&gc); } - -/* - * argv_prep_format - prepare argv format string for further processing +/** + * Prepares argv format string for further processing * - * Individual argument must be separated by space. Ignores leading and trailing spaces. - * Consecutive spaces count as one. Returns prepared format string, with space replaced - * by delim and adds the number of arguments to the count parameter. + * Individual argument must be separated by space. Ignores leading and + * trailing spaces. Consecutive spaces count as one. Returns prepared + * format string, with space replaced by delim and adds the number of + * arguments to the count parameter. + * + * @param *format Pointer to a the format string to process + * @param delim Char with the delimiter to use + * @param *count size_t pointer used to return the number of + * tokens (argument slots) found in the format string. + * @param *gc Pointer to a gc_arena managed buffer. + * + * @returns Returns a parsed format string (char *), together with the + * number of tokens parts found (via *count). The result string + * is allocated within the gc_arena managed buffer. If the + * gc_arena pointer is NULL, the returned string must be explicitly + * free()d to avoid memory leaks. */ static char * -argv_prep_format(const char *format, const char delim, size_t *count, struct gc_arena *gc) +argv_prep_format(const char *format, const char delim, size_t *count, + struct gc_arena *gc) { int i, j; char *f; @@ -213,15 +331,28 @@ argv_prep_format(const char *format, const char delim, size_t *count, struct gc_ return f; } -/* - * argv_printf_arglist - create a struct argv from a format string +/** + * Create a struct argv based on a format string * - * Instead of parsing the format string ourselves place delimiters via argv_prep_format() - * before we let libc's printf() do the parsing. Then split the resulting string at the - * injected delimiters. + * Instead of parsing the format string ourselves place delimiters via + * argv_prep_format() before we let libc's printf() do the parsing. + * Then split the resulting string at the injected delimiters. + * + * @param *argres Valid pointer to a struct argv where the resulting parsed + * arguments, based on the format string. + * @param *format Char* string with a printf() compliant format string + * @param arglist A va_list with the arguments to be consumed by the format + * string + * + * @returns Returns true if the parsing and processing was successfully. If + * the resulting number of arguments does not match the expected + * number of arguments (based on the format string), it is + * considered a failure, which returns false. This can happen if + * the ASCII Group Separator (GS - 0x1D) is put into the arguments + * list or format string. */ static bool -argv_printf_arglist(struct argv *a, const char *format, va_list arglist) +argv_printf_arglist(struct argv *argres, const char *format, va_list arglist) { struct gc_arena gc = gc_new(); const char delim = 0x1D; /* ASCII Group Separator (GS) */ @@ -231,14 +362,19 @@ argv_printf_arglist(struct argv *a, const char *format, va_list arglist) va_list tmplist; int len; - argc = a->argc; + argc = argres->argc; f = argv_prep_format(format, delim, &argc, &gc); if (f == NULL) { goto out; } - /* determine minimum buffer size */ + /* + * Determine minimum buffer size. + * + * With C99, vsnprintf(NULL, 0, ...) will return the number of bytes + * it would have written, had the buffer been large enough. + */ va_copy(tmplist, arglist); len = vsnprintf(NULL, 0, f, tmplist); va_end(tmplist); @@ -248,7 +384,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, &a->gc); + buf = gc_malloc(size, false, &argres->gc); len = vsnprintf(buf, size, f, arglist); if (len < 0 || len >= size) { @@ -260,16 +396,16 @@ argv_printf_arglist(struct argv *a, const char *format, va_list arglist) while (end) { *end = '\0'; - argv_append(a, buf); + argv_append(argres, buf); buf = end + 1; end = strchr(buf, delim); } - argv_append(a, buf); + argv_append(argres, buf); - if (a->argc != argc) + if (argres->argc != argc) { /* Someone snuck in a GS (0x1D), fail gracefully */ - argv_reset(a); + argv_reset(argres); goto out; } @@ -280,51 +416,87 @@ out: return res; } - - +/** + * printf() variant which populates a struct argv. It processes the + * format string with the provided arguments. For each space separator found + * in the format string, a new argument will be added to the resulting + * struct argv. + * + * This will always reset and ensure the result is based on a pristine + * struct argv. + * + * @param *argres Valid pointer to a struct argv where the result will be put. + * @param *format printf() compliant (char *) format string. + * + * @returns Returns true if the parsing was successful. See + * argv_printf_arglist() for more details. The parsed result will + * be put into argres. + */ bool -argv_printf(struct argv *a, const char *format, ...) +argv_printf(struct argv *argres, const char *format, ...) { bool res; va_list arglist; va_start(arglist, format); - argv_reset(a); - res = argv_printf_arglist(a, format, arglist); + argv_reset(argres); + res = argv_printf_arglist(argres, format, arglist); va_end(arglist); return res; } +/** + * printf() inspired argv concatenation. Adds arguments to an existing + * struct argv and populets the argument slots based on the printf() based + * format string. + * + * @param *argres Valid pointer to a struct argv where the result will be put. + * @param *format printf() compliant (char *) format string. + * + * @returns Returns true if the parsing was successful. See + * argv_printf_arglist() for more details. The parsed result will + * be put into argres. + */ bool -argv_printf_cat(struct argv *a, const char *format, ...) +argv_printf_cat(struct argv *argres, const char *format, ...) { bool res; va_list arglist; va_start(arglist, format); - res = argv_printf_arglist(a, format, arglist); + res = argv_printf_arglist(argres, format, arglist); va_end(arglist); return res; } +/** + * Parses a command string, tokenizes it and puts each element into a separate + * struct argv argument slot. + * + * @params *argres Valid pointer to a struct argv where the parsed result + * will be found. + * @params *cmdstr Char * based string to parse + * + */ void -argv_parse_cmd(struct argv *a, const char *s) +argv_parse_cmd(struct argv *argres, const char *cmdstr) { int nparms; char *parms[MAX_PARMS + 1]; - argv_reset(a); + argv_reset(argres); - nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0, D_ARGV_PARSE_CMD, &a->gc); + nparms = parse_line(cmdstr, parms, MAX_PARMS, "SCRIPT-ARGV", 0, + D_ARGV_PARSE_CMD, &argres->gc); if (nparms) { int i; for (i = 0; i < nparms; ++i) { - argv_append(a, parms[i]); + argv_append(argres, parms[i]); } } else { - argv_append(a, string_alloc(s, &a->gc)); + argv_append(argres, string_alloc(cmdstr, &argres->gc)); } }