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 the argv_reset functionality so that it can
> be used to actually reset the argv without re-allocation.
>
> Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
> ---
> src/openvpn/argv.c | 53
> ++++++++++++++++++------------------
> 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 | 32 ++++++++++++----------
> tests/unit_tests/openvpn/test_argv.c | 41 +++++++++++++++++++---------
> 13 files changed, 94 insertions(+), 85 deletions(-)
>
> diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
> index afe8efff..419b1dc6 100644
> --- a/src/openvpn/argv.c
> +++ b/src/openvpn/argv.c
> @@ -40,11 +40,30 @@
> #include "options.h"
>
> static void
> +argv_extend(struct argv *a, const size_t newcap)
> +{
> + if (newcap > a->capacity)
> + {
> + char **newargv;
> + size_t i;
> + ALLOC_ARRAY_CLEAR(newargv, char *, newcap);
Did we agree on using the intern gc_arena for this array?
Or not? I remember we talked about ALLOC_ARRAY_CLEAR_GC()
instead.
[...snip...]
> static void
> @@ -133,14 +145,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);
> }
Hmmm .... do we _really_ need argv_str()?
$ git grep argv_str
src/openvpn/argv.c:argv_str(const struct argv *a, struct gc_arena *gc, const
unsigned int flags)
src/openvpn/argv.c: msg(msglev, "%s", argv_str(a, &gc, 0));
src/openvpn/argv.c: msg(msglev, "%s: %s", prefix, argv_str(a, &gc, 0));
src/openvpn/argv.h:const char *argv_str(const struct argv *a, struct gc_arena
*gc, const unsigned int flags);
(ignoring unit tests).
Or if we want it for convenience for those two callers in argv.c, remove it
from the
header and make it static inline. But I'm still not really convinced we really
need
it; as the usage is limited to within the argv.c code base.
Otherwise, this looks good. Once we have figured out these two comments, this
is
fairly straight-forward to include.
--
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
