From 7aa6c12a4424d00ea0add0a849f8a5b31a2de6a1 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Sat, 28 Aug 2010 20:14:36 +0200 Subject: [PATCH 1/9] Clean-up: Remove pthread and mutex locking code This code was not activated at all, and hard coded as disabled in syshead.h with this code snippet: /* * Pthread support is currently experimental (and quite unfinished). */ #if 1 /* JYFIXME -- if defined, disable pthread */ #undef USE_PTHREAD #endif So no matter if --enable-pthread when running ./configure or not, this feature was never enabled in reality. Further, by removing the blocker code above made OpenVPN uncompilable in the current state. As the threading part needs to be completely rewritten and pthreading will not be supported in OpenVPN 2.x, removing this code seems most reasonable. In addition, a lot of mutex locking code was also removed, as they were practically NOP functions, due to pthreading being forcefully disabled Signed-off-by: David Sommerseth Acked-by: James Yonan --- Makefile.am | 1 - acinclude.m4 | 224 ------------------------------------------- buffer.c | 1 - buffer.h | 1 - config-win32.h.in | 5 - configure.ac | 38 +------- crypto.c | 3 - error.c | 39 -------- error.h | 17 ---- init.c | 50 ---------- list.c | 13 --- list.h | 8 -- mbuf.c | 12 --- mbuf.h | 1 - misc.c | 23 ----- multi.c | 1 - multi.h | 2 - options.c | 36 ------- options.h | 5 - otime.c | 2 - otime.h | 1 - perf.c | 4 - plugin.c | 4 - pool.h | 1 - schedule.c | 4 - schedule.h | 8 -- socket.c | 7 -- ssl.c | 1 - ssl.h | 1 - syshead.h | 18 ---- thread.c | 156 ------------------------------ thread.h | 235 ---------------------------------------------- 32 files changed, 1 insertion(+), 921 deletions(-) delete mode 100644 thread.c delete mode 100644 thread.h diff --git a/Makefile.am b/Makefile.am index 76573d11..c7b91c8b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -137,7 +137,6 @@ openvpn_SOURCES = \ ssl.c ssl.h \ status.c status.h \ syshead.h \ - thread.c thread.h \ tun.c tun.h \ win32.h win32.c \ cryptoapi.h cryptoapi.c diff --git a/acinclude.m4 b/acinclude.m4 index f099de53..f037484c 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -129,227 +129,3 @@ AC_DEFUN([TYPE_SOCKLEN_T], [#include #include ]) ]) - -dnl @synopsis ACX_PTHREAD([ACTION-IF-FOUND[, ACTION-IF-NOT-FOUND]]) -dnl -dnl This macro figures out how to build C programs using POSIX -dnl threads. It sets the PTHREAD_LIBS output variable to the threads -dnl library and linker flags, and the PTHREAD_CFLAGS output variable -dnl to any special C compiler flags that are needed. (The user can also -dnl force certain compiler flags/libs to be tested by setting these -dnl environment variables.) -dnl -dnl Also sets PTHREAD_CC to any special C compiler that is needed for -dnl multi-threaded programs (defaults to the value of CC otherwise). -dnl (This is necessary on AIX to use the special cc_r compiler alias.) -dnl -dnl If you are only building threads programs, you may wish to -dnl use these variables in your default LIBS, CFLAGS, and CC: -dnl -dnl LIBS="$PTHREAD_LIBS $LIBS" -dnl CFLAGS="$CFLAGS $PTHREAD_CFLAGS" -dnl CC="$PTHREAD_CC" -dnl -dnl In addition, if the PTHREAD_CREATE_JOINABLE thread-attribute -dnl constant has a nonstandard name, defines PTHREAD_CREATE_JOINABLE -dnl to that name (e.g. PTHREAD_CREATE_UNDETACHED on AIX). -dnl -dnl ACTION-IF-FOUND is a list of shell commands to run if a threads -dnl library is found, and ACTION-IF-NOT-FOUND is a list of commands -dnl to run it if it is not found. If ACTION-IF-FOUND is not specified, -dnl the default action will define HAVE_PTHREAD. -dnl -dnl Please let the authors know if this macro fails on any platform, -dnl or if you have any other suggestions or comments. This macro was -dnl based on work by SGJ on autoconf scripts for FFTW (www.fftw.org) -dnl (with help from M. Frigo), as well as ac_pthread and hb_pthread -dnl macros posted by AFC to the autoconf macro repository. We are also -dnl grateful for the helpful feedback of numerous users. -dnl -dnl @author Steven G. Johnson and Alejandro Forero Cuervo - -AC_DEFUN([ACX_PTHREAD], [ -AC_REQUIRE([AC_CANONICAL_HOST]) -acx_pthread_ok=no - -# We used to check for pthread.h first, but this fails if pthread.h -# requires special compiler flags (e.g. on True64 or Sequent). -# It gets checked for in the link test anyway. - -# First of all, check if the user has set any of the PTHREAD_LIBS, -# etcetera environment variables, and if threads linking works using -# them: -if test x"$PTHREAD_LIBS$PTHREAD_CFLAGS" != x; then - save_CFLAGS="$CFLAGS" - CFLAGS="$CFLAGS $PTHREAD_CFLAGS" - save_LIBS="$LIBS" - LIBS="$PTHREAD_LIBS $LIBS" - AC_MSG_CHECKING([for pthread_join in LIBS=$PTHREAD_LIBS with CFLAGS=$PTHREAD_CFLAGS]) - AC_TRY_LINK_FUNC(pthread_join, acx_pthread_ok=yes) - AC_MSG_RESULT($acx_pthread_ok) - if test x"$acx_pthread_ok" = xno; then - PTHREAD_LIBS="" - PTHREAD_CFLAGS="" - fi - LIBS="$save_LIBS" - CFLAGS="$save_CFLAGS" -fi - -# We must check for the threads library under a number of different -# names; the ordering is very important because some systems -# (e.g. DEC) have both -lpthread and -lpthreads, where one of the -# libraries is broken (non-POSIX). - -# Create a list of thread flags to try. Items starting with a "-" are -# C compiler flags, and other items are library names, except for "none" -# which indicates that we try without any flags at all. - -acx_pthread_flags="pthreads none -Kthread -kthread lthread -pthread -pthreads -mthreads pthread --thread-safe -mt" - -# The ordering *is* (sometimes) important. Some notes on the -# individual items follow: - -# pthreads: AIX (must check this before -lpthread) -# none: in case threads are in libc; should be tried before -Kthread and -# other compiler flags to prevent continual compiler warnings -# -Kthread: Sequent (threads in libc, but -Kthread needed for pthread.h) -# -kthread: FreeBSD kernel threads (preferred to -pthread since SMP-able) -# lthread: LinuxThreads port on FreeBSD (also preferred to -pthread) -# -pthread: Linux/gcc (kernel threads), BSD/gcc (userland threads) -# -pthreads: Solaris/gcc -# -mthreads: Mingw32/gcc, Lynx/gcc -# -mt: Sun Workshop C (may only link SunOS threads [-lthread], but it -# doesn't hurt to check since this sometimes defines pthreads too; -# also defines -D_REENTRANT) -# pthread: Linux, etcetera -# --thread-safe: KAI C++ - -case "$target" in - *solaris*) - - # On Solaris (at least, for some versions), libc contains stubbed - # (non-functional) versions of the pthreads routines, so link-based - # tests will erroneously succeed. (We need to link with -pthread or - # -lpthread.) (The stubs are missing pthread_cleanup_push, or rather - # a function called by this macro, so we could check for that, but - # who knows whether they'll stub that too in a future libc.) So, - # we'll just look for -pthreads and -lpthread first: - - acx_pthread_flags="-pthread -pthreads pthread -mt $acx_pthread_flags" - ;; -esac - -if test x"$acx_pthread_ok" = xno; then -for flag in $acx_pthread_flags; do - - case $flag in - none) - AC_MSG_CHECKING([whether pthreads work without any flags]) - ;; - - -*) - AC_MSG_CHECKING([whether pthreads work with $flag]) - PTHREAD_CFLAGS="$flag" - ;; - - *) - AC_MSG_CHECKING([for the pthreads library -l$flag]) - PTHREAD_LIBS="-l$flag" - ;; - esac - - save_LIBS="$LIBS" - save_CFLAGS="$CFLAGS" - LIBS="$PTHREAD_LIBS $LIBS" - CFLAGS="$CFLAGS $PTHREAD_CFLAGS" - - # Check for various functions. We must include pthread.h, - # since some functions may be macros. (On the Sequent, we - # need a special flag -Kthread to make this header compile.) - # We check for pthread_join because it is in -lpthread on IRIX - # while pthread_create is in libc. We check for pthread_attr_init - # due to DEC craziness with -lpthreads. We check for - # pthread_cleanup_push because it is one of the few pthread - # functions on Solaris that doesn't have a non-functional libc stub. - # We try pthread_create on general principles. - AC_TRY_LINK([#include ], - [pthread_t th; pthread_join(th, 0); - pthread_attr_init(0); pthread_cleanup_push(0, 0); - pthread_create(0,0,0,0); pthread_cleanup_pop(0); ], - [acx_pthread_ok=yes]) - - LIBS="$save_LIBS" - CFLAGS="$save_CFLAGS" - - AC_MSG_RESULT($acx_pthread_ok) - if test "x$acx_pthread_ok" = xyes; then - break; - fi - - PTHREAD_LIBS="" - PTHREAD_CFLAGS="" -done -fi - -# Various other checks: -if test "x$acx_pthread_ok" = xyes; then - save_LIBS="$LIBS" - LIBS="$PTHREAD_LIBS $LIBS" - save_CFLAGS="$CFLAGS" - CFLAGS="$CFLAGS $PTHREAD_CFLAGS" - - # Detect AIX lossage: threads are created detached by default - # and the JOINABLE attribute has a nonstandard name (UNDETACHED). - AC_MSG_CHECKING([for joinable pthread attribute]) - AC_TRY_LINK([#include ], - [int attr=PTHREAD_CREATE_JOINABLE;], - ok=PTHREAD_CREATE_JOINABLE, ok=unknown) - if test x"$ok" = xunknown; then - AC_TRY_LINK([#include ], - [int attr=PTHREAD_CREATE_UNDETACHED;], - ok=PTHREAD_CREATE_UNDETACHED, ok=unknown) - fi - if test x"$ok" != xPTHREAD_CREATE_JOINABLE; then - AC_DEFINE(PTHREAD_CREATE_JOINABLE, $ok, - [Define to the necessary symbol if this constant - uses a non-standard name on your system.]) - fi - AC_MSG_RESULT(${ok}) - if test x"$ok" = xunknown; then - AC_MSG_WARN([we do not know how to create joinable pthreads]) - fi - - AC_MSG_CHECKING([if more special flags are required for pthreads]) - flag=no - case "$target" in - *-aix* | *-freebsd*) flag="-D_THREAD_SAFE";; - *solaris* | alpha*-osf* | *linux*) flag="-D_REENTRANT";; - esac - AC_MSG_RESULT(${flag}) - if test "x$flag" != xno; then - PTHREAD_CFLAGS="$flag $PTHREAD_CFLAGS" - fi - - LIBS="$save_LIBS" - CFLAGS="$save_CFLAGS" - - # More AIX lossage: must compile with cc_r - AC_CHECK_PROG(PTHREAD_CC, cc_r, cc_r, ${CC}) -else - PTHREAD_CC="$CC" -fi - -AC_SUBST(PTHREAD_LIBS) -AC_SUBST(PTHREAD_CFLAGS) -AC_SUBST(PTHREAD_CC) - -# Finally, execute ACTION-IF-FOUND/ACTION-IF-NOT-FOUND: -if test x"$acx_pthread_ok" = xyes; then - ifelse([$1],,AC_DEFINE(HAVE_PTHREAD,1,[Define if you have POSIX threads libraries and header files.]),[$1]) - : -else - acx_pthread_ok=no - $2 -fi - -])dnl ACX_PTHREAD diff --git a/buffer.c b/buffer.c index d448e5d7..e9ae21a1 100644 --- a/buffer.c +++ b/buffer.c @@ -28,7 +28,6 @@ #include "buffer.h" #include "error.h" #include "mtu.h" -#include "thread.h" #include "memdbg.h" diff --git a/buffer.h b/buffer.h index b046a5cf..399a31be 100644 --- a/buffer.h +++ b/buffer.h @@ -26,7 +26,6 @@ #define BUFFER_H #include "basic.h" -#include "thread.h" #define BUF_SIZE_MAX 1000000 diff --git a/config-win32.h.in b/config-win32.h.in index 6f699bbb..a81de0cb 100644 --- a/config-win32.h.in +++ b/config-win32.h.in @@ -298,11 +298,6 @@ typedef unsigned long in_addr_t; /* Route command */ #define ROUTE_PATH "route" -/* Windows doesn't support PTHREAD yet */ -#ifdef USE_PTHREAD -#error The Windows version of OpenVPN does not support PTHREAD yet -#endif - #ifdef _MSC_VER /* MSVC++ hacks */ #include diff --git a/configure.ac b/configure.ac index 116ff7c9..9d8a5154 100644 --- a/configure.ac +++ b/configure.ac @@ -153,12 +153,6 @@ AC_ARG_ENABLE(small, [SMALL="no"] ) -AC_ARG_ENABLE(pthread, - [ --enable-pthread Enable pthread support (Experimental for OpenVPN 2.0)], - [PTHREAD="$enableval"], - [PTHREAD="no"] -) - AC_ARG_ENABLE(password-save, [ --enable-password-save Allow --askpass and --auth-user-pass passwords to be read from a file], [PASSWORD_SAVE="$enableval"], @@ -565,32 +559,6 @@ if test "$MEMCHECK" = "valgrind"; then ) fi -dnl -dnl check for pthread library -dnl - -if test "$PTHREAD" = "yes"; then - AC_CHECKING([for pthread support]) - AC_MSG_RESULT([********* WARNING: pthread support is experimental for OpenVPN 2.0]) - ACX_PTHREAD( - [ - case "$target" in - *openbsd*) - AC_MSG_RESULT([WARNING: pthread support on OpenBSD is unstable!]) - CFLAGS="$CFLAGS -pthread" - ;; - esac - LIBS="$PTHREAD_LIBS $LIBS" - CFLAGS="$CFLAGS $PTHREAD_CFLAGS" - CC="$PTHREAD_CC" - AC_DEFINE(USE_PTHREAD, 1, [Use pthread-based multithreading]) - ], - [ - AC_MSG_RESULT([I don't know how to build with pthread support on this platform.]) - AC_MSG_ERROR([try ./configure --disable-pthread]) - ]) -fi - dnl dnl check for dmalloc library dnl @@ -600,11 +568,7 @@ if test "$MEMCHECK" = "dmalloc"; then AC_CHECK_HEADER(dmalloc.h, [AC_CHECK_LIB(dmalloc, malloc, [ - if test "$PTHREAD" = "yes"; then - OPENVPN_ADD_LIBS(-ldmallocth) - else - OPENVPN_ADD_LIBS(-ldmalloc) - fi + OPENVPN_ADD_LIBS(-ldmalloc) AC_DEFINE(DMALLOC, 1, [Use dmalloc memory debugging library]) ], [AC_MSG_ERROR([dmalloc library not found.])] diff --git a/crypto.c b/crypto.c index 444f036a..c73db931 100644 --- a/crypto.c +++ b/crypto.c @@ -29,7 +29,6 @@ #include "crypto.h" #include "error.h" #include "misc.h" -#include "thread.h" #include "memdbg.h" @@ -1702,7 +1701,6 @@ prng_bytes (uint8_t *output, int len) { EVP_MD_CTX ctx; const int md_size = EVP_MD_size (nonce_md); - mutex_lock_static (L_PRNG); while (len > 0) { unsigned int outlen = 0; @@ -1716,7 +1714,6 @@ prng_bytes (uint8_t *output, int len) output += blen; len -= blen; } - mutex_unlock_static (L_PRNG); } else RAND_bytes (output, len); diff --git a/error.c b/error.c index 603796a1..2dfb9789 100644 --- a/error.c +++ b/error.c @@ -26,7 +26,6 @@ #include "error.h" #include "buffer.h" -#include "thread.h" #include "misc.h" #include "win32.h" #include "socket.h" @@ -229,8 +228,6 @@ void x_msg (const unsigned int flags, const char *format, ...) gc_init (&gc); - mutex_lock_static (L_MSG); - m1 = (char *) gc_malloc (ERR_BUF_SIZE, false, &gc); m2 = (char *) gc_malloc (ERR_BUF_SIZE, false, &gc); @@ -330,22 +327,12 @@ void x_msg (const unsigned int flags, const char *format, ...) } else { -#ifdef USE_PTHREAD - fprintf (fp, "%s [%d] %s%s%s%s", - time_string (0, 0, show_usec, &gc), - (int) openvpn_thread_self (), - prefix, - prefix_sep, - m1, - (flags&M_NOLF) ? "" : "\n"); -#else fprintf (fp, "%s %s%s%s%s", time_string (0, 0, show_usec, &gc), prefix, prefix_sep, m1, (flags&M_NOLF) ? "" : "\n"); -#endif } fflush(fp); ++x_msg_line_num; @@ -355,8 +342,6 @@ void x_msg (const unsigned int flags, const char *format, ...) if (flags & M_FATAL) msg (M_INFO, "Exiting"); - mutex_unlock_static (L_MSG); - if (flags & M_FATAL) openvpn_exit (OPENVPN_EXIT_STATUS_ERROR); /* exit point */ @@ -645,36 +630,12 @@ x_check_status (int status, */ const char *x_msg_prefix; /* GLOBAL */ -#ifdef USE_PTHREAD -pthread_key_t x_msg_prefix_key; /* GLOBAL */ -#endif - /* * Allow MSG to be redirected through a virtual_output object */ const struct virtual_output *x_msg_virtual_output; /* GLOBAL */ -/* - * Init thread-local variables - */ - -void -msg_thread_init (void) -{ -#ifdef USE_PTHREAD - ASSERT (!pthread_key_create (&x_msg_prefix_key, NULL)); -#endif -} - -void -msg_thread_uninit (void) -{ -#ifdef USE_PTHREAD - pthread_key_delete (x_msg_prefix_key); -#endif -} - /* * Exiting. */ diff --git a/error.h b/error.h index 5c6659bd..d1755331 100644 --- a/error.h +++ b/error.h @@ -26,7 +26,6 @@ #define ERROR_H #include "basic.h" -#include "thread.h" /* #define ABORT_ON_ERROR */ @@ -282,34 +281,18 @@ set_check_status_error_delay (unsigned int milliseconds) extern const char *x_msg_prefix; -#ifdef USE_PTHREAD -extern pthread_key_t x_msg_prefix_key; -#endif - void msg_thread_init (void); void msg_thread_uninit (void); static inline void msg_set_prefix (const char *prefix) { -#ifdef USE_PTHREAD - if (openvpn_thread_enabled ()) - { - ASSERT (!pthread_setspecific (x_msg_prefix_key, prefix)); - } - else -#endif x_msg_prefix = prefix; } static inline const char * msg_get_prefix (void) { -#ifdef USE_PTHREAD - if (openvpn_thread_enabled ()) - return (const char *) pthread_getspecific (x_msg_prefix_key); - else -#endif return x_msg_prefix; } diff --git a/init.c b/init.c index f765d9d7..b4a03848 100644 --- a/init.c +++ b/init.c @@ -506,8 +506,6 @@ init_static (void) void uninit_static (void) { - openvpn_thread_cleanup (); - #ifdef USE_CRYPTO free_ssl_lib (); #endif @@ -3280,23 +3278,6 @@ close_context (struct context *c, int sig, unsigned int flags) #ifdef USE_CRYPTO -static void -test_malloc (void) -{ - int i, j; - msg (M_INFO, "Multithreaded malloc test..."); - for (i = 0; i < 25; ++i) - { - struct gc_arena gc = gc_new (); - const int limit = get_random () & 0x03FF; - for (j = 0; j < limit; ++j) - { - gc_malloc (get_random () & 0x03FF, false, &gc); - } - gc_free (&gc); - } -} - /* * Do a loopback test * on the crypto subsystem. @@ -3306,50 +3287,19 @@ test_crypto_thread (void *arg) { struct context *c = (struct context *) arg; const struct options *options = &c->options; -#if defined(USE_PTHREAD) - struct context *child = NULL; - openvpn_thread_t child_id = 0; -#endif ASSERT (options->test_crypto); init_verb_mute (c, IVM_LEVEL_1); context_init_1 (c); do_init_crypto_static (c, 0); -#if defined(USE_PTHREAD) - { - if (c->first_time && options->n_threads > 1) - { - if (options->n_threads > 2) - msg (M_FATAL, "ERROR: --test-crypto option only works with --threads set to 1 or 2"); - openvpn_thread_init (); - ALLOC_OBJ (child, struct context); - context_clear (child); - child->options = *options; - options_detach (&child->options); - child->first_time = false; - child_id = openvpn_thread_create (test_crypto_thread, (void *) child); - } - } -#endif frame_finalize_options (c, options); -#if defined(USE_PTHREAD) - if (options->n_threads == 2) - test_malloc (); -#endif - test_crypto (&c->c2.crypto_options, &c->c2.frame); key_schedule_free (&c->c1.ks, true); packet_id_free (&c->c2.packet_id); -#if defined(USE_PTHREAD) - if (c->first_time && options->n_threads > 1) - openvpn_thread_join (child_id); - if (child) - free (child); -#endif context_gc_free (c); return NULL; } diff --git a/list.c b/list.c index 7e57782a..d1e5a157 100644 --- a/list.c +++ b/list.c @@ -52,7 +52,6 @@ hash_init (const int n_buckets, { struct hash_bucket *b = &h->buckets[i]; b->list = NULL; - mutex_init (&b->mutex); } return h; } @@ -66,7 +65,6 @@ hash_free (struct hash *hash) struct hash_bucket *b = &hash->buckets[i]; struct hash_element *he = b->list; - mutex_destroy (&b->mutex); while (he) { struct hash_element *next = he->next; @@ -148,7 +146,6 @@ hash_add (struct hash *hash, const void *key, void *value, bool replace) hv = hash_value (hash, key); bucket = &hash->buckets[hv & hash->mask]; - mutex_lock (&bucket->mutex); if ((he = hash_lookup_fast (hash, bucket, key, hv))) /* already exists? */ { @@ -164,8 +161,6 @@ hash_add (struct hash *hash, const void *key, void *value, bool replace) ret = true; } - mutex_unlock (&bucket->mutex); - return ret; } @@ -257,10 +252,6 @@ hash_iterator_init (struct hash *hash, static inline void hash_iterator_lock (struct hash_iterator *hi, struct hash_bucket *b) { - if (hi->autolock) - { - mutex_lock (&b->mutex); - } hi->bucket = b; hi->last = NULL; hi->bucket_marked = false; @@ -276,10 +267,6 @@ hash_iterator_unlock (struct hash_iterator *hi) hash_remove_marked (hi->hash, hi->bucket); hi->bucket_marked = false; } - if (hi->autolock) - { - mutex_unlock (&hi->bucket->mutex); - } hi->bucket = NULL; hi->last = NULL; } diff --git a/list.h b/list.h index 3f4a9317..138e894d 100644 --- a/list.h +++ b/list.h @@ -40,7 +40,6 @@ /*#define LIST_TEST*/ #include "basic.h" -#include "thread.h" #include "buffer.h" #define hashsize(n) ((uint32_t)1<<(n)) @@ -56,7 +55,6 @@ struct hash_element struct hash_bucket { - MUTEX_DEFINE (mutex); struct hash_element *list; }; @@ -152,13 +150,11 @@ hash_bucket (struct hash *hash, uint32_t hv) static inline void hash_bucket_lock (struct hash_bucket *bucket) { - mutex_lock (&bucket->mutex); } static inline void hash_bucket_unlock (struct hash_bucket *bucket) { - mutex_unlock (&bucket->mutex); } static inline void * @@ -168,11 +164,9 @@ hash_lookup_lock (struct hash *hash, const void *key, uint32_t hv) struct hash_element *he; struct hash_bucket *bucket = &hash->buckets[hv & hash->mask]; - mutex_lock (&bucket->mutex); he = hash_lookup_fast (hash, bucket, key, hv); if (he) ret = he->value; - mutex_unlock (&bucket->mutex); return ret; } @@ -211,9 +205,7 @@ hash_remove (struct hash *hash, const void *key) hv = hash_value (hash, key); bucket = &hash->buckets[hv & hash->mask]; - mutex_lock (&bucket->mutex); ret = hash_remove_fast (hash, bucket, key, hv); - mutex_unlock (&bucket->mutex); return ret; } diff --git a/mbuf.c b/mbuf.c index 55960b3e..8ffda00c 100644 --- a/mbuf.c +++ b/mbuf.c @@ -38,7 +38,6 @@ mbuf_init (unsigned int size) { struct mbuf_set *ret; ALLOC_OBJ_CLEAR (ret, struct mbuf_set); - mutex_init (&ret->mutex); ret->capacity = adjust_power_of_2 (size); ALLOC_ARRAY (ret->array, struct mbuf_item, ret->capacity); return ret; @@ -56,7 +55,6 @@ mbuf_free (struct mbuf_set *ms) mbuf_free_buf (item->buffer); } free (ms->array); - mutex_destroy (&ms->mutex); free (ms); } } @@ -89,7 +87,6 @@ void mbuf_add_item (struct mbuf_set *ms, const struct mbuf_item *item) { ASSERT (ms); - mutex_lock (&ms->mutex); if (ms->len == ms->capacity) { struct mbuf_item rm; @@ -104,7 +101,6 @@ mbuf_add_item (struct mbuf_set *ms, const struct mbuf_item *item) if (++ms->len > ms->max_queued) ms->max_queued = ms->len; ++item->buffer->refcount; - mutex_unlock (&ms->mutex); } bool @@ -113,8 +109,6 @@ mbuf_extract_item (struct mbuf_set *ms, struct mbuf_item *item, const bool lock) bool ret = false; if (ms) { - if (lock) - mutex_lock (&ms->mutex); while (ms->len) { *item = ms->array[ms->head]; @@ -126,8 +120,6 @@ mbuf_extract_item (struct mbuf_set *ms, struct mbuf_item *item, const bool lock) break; } } - if (lock) - mutex_unlock (&ms->mutex); } return ret; } @@ -139,7 +131,6 @@ mbuf_peek_dowork (struct mbuf_set *ms) if (ms) { int i; - mutex_lock (&ms->mutex); for (i = 0; i < (int) ms->len; ++i) { struct mbuf_item *item = &ms->array[MBUF_INDEX(ms->head, i, ms->capacity)]; @@ -149,7 +140,6 @@ mbuf_peek_dowork (struct mbuf_set *ms) break; } } - mutex_unlock (&ms->mutex); } return ret; } @@ -160,7 +150,6 @@ mbuf_dereference_instance (struct mbuf_set *ms, struct multi_instance *mi) if (ms) { int i; - mutex_lock (&ms->mutex); for (i = 0; i < (int) ms->len; ++i) { struct mbuf_item *item = &ms->array[MBUF_INDEX(ms->head, i, ms->capacity)]; @@ -172,7 +161,6 @@ mbuf_dereference_instance (struct mbuf_set *ms, struct multi_instance *mi) msg (D_MBUF, "MBUF: dereferenced queued packet"); } } - mutex_unlock (&ms->mutex); } } diff --git a/mbuf.h b/mbuf.h index 21435f87..7be45559 100644 --- a/mbuf.h +++ b/mbuf.h @@ -58,7 +58,6 @@ struct mbuf_item struct mbuf_set { - MUTEX_DEFINE (mutex); unsigned int head; unsigned int len; unsigned int capacity; diff --git a/misc.c b/misc.c index 9fce0c83..815c057c 100644 --- a/misc.c +++ b/misc.c @@ -28,7 +28,6 @@ #include "misc.h" #include "tun.h" #include "error.h" -#include "thread.h" #include "otime.h" #include "plugin.h" #include "options.h" @@ -639,9 +638,7 @@ strerror_ts (int errnum, struct gc_arena *gc) #ifdef HAVE_STRERROR struct buffer out = alloc_buf_gc (256, gc); - mutex_lock_static (L_STRERR); buf_printf (&out, "%s", openvpn_strerror (errnum, gc)); - mutex_unlock_static (L_STRERR); return BSTR (&out); #else return "[error string unavailable]"; @@ -779,18 +776,15 @@ struct env_set * env_set_create (struct gc_arena *gc) { struct env_set *es; - mutex_lock_static (L_ENV_SET); ALLOC_OBJ_CLEAR_GC (es, struct env_set, gc); es->list = NULL; es->gc = gc; - mutex_unlock_static (L_ENV_SET); return es; } void env_set_destroy (struct env_set *es) { - mutex_lock_static (L_ENV_SET); if (es && es->gc == NULL) { struct env_item *e = es->list; @@ -803,7 +797,6 @@ env_set_destroy (struct env_set *es) } free (es); } - mutex_unlock_static (L_ENV_SET); } bool @@ -812,9 +805,7 @@ env_set_del (struct env_set *es, const char *str) bool ret; ASSERT (es); ASSERT (str); - mutex_lock_static (L_ENV_SET); ret = env_set_del_nolock (es, str); - mutex_unlock_static (L_ENV_SET); return ret; } @@ -823,9 +814,7 @@ env_set_add (struct env_set *es, const char *str) { ASSERT (es); ASSERT (str); - mutex_lock_static (L_ENV_SET); env_set_add_nolock (es, str); - mutex_unlock_static (L_ENV_SET); } void @@ -838,7 +827,6 @@ env_set_print (int msglevel, const struct env_set *es) if (es) { - mutex_lock_static (L_ENV_SET); e = es->list; i = 0; @@ -849,7 +837,6 @@ env_set_print (int msglevel, const struct env_set *es) ++i; e = e->next; } - mutex_unlock_static (L_ENV_SET); } } } @@ -863,14 +850,12 @@ env_set_inherit (struct env_set *es, const struct env_set *src) if (src) { - mutex_lock_static (L_ENV_SET); e = src->list; while (e) { env_set_add_nolock (es, e->string); e = e->next; } - mutex_unlock_static (L_ENV_SET); } } @@ -882,7 +867,6 @@ env_set_add_to_environment (const struct env_set *es) struct gc_arena gc = gc_new (); const struct env_item *e; - mutex_lock_static (L_ENV_SET); e = es->list; while (e) @@ -895,7 +879,6 @@ env_set_add_to_environment (const struct env_set *es) e = e->next; } - mutex_unlock_static (L_ENV_SET); gc_free (&gc); } } @@ -908,7 +891,6 @@ env_set_remove_from_environment (const struct env_set *es) struct gc_arena gc = gc_new (); const struct env_item *e; - mutex_lock_static (L_ENV_SET); e = es->list; while (e) @@ -921,7 +903,6 @@ env_set_remove_from_environment (const struct env_set *es) e = e->next; } - mutex_unlock_static (L_ENV_SET); gc_free (&gc); } } @@ -1040,12 +1021,10 @@ setenv_str_ex (struct env_set *es, char *str = construct_name_value (name_tmp, val_tmp, NULL); int status; - mutex_lock_static (L_PUTENV); status = putenv (str); /*msg (M_INFO, "PUTENV '%s'", str);*/ if (!status) manage_env (str); - mutex_unlock_static (L_PUTENV); if (status) msg (M_WARN | M_ERRNO, "putenv('%s') failed", str); } @@ -1173,9 +1152,7 @@ create_temp_filename (const char *directory, const char *prefix, struct gc_arena static unsigned int counter; struct buffer fname = alloc_buf_gc (256, gc); - mutex_lock_static (L_CREATE_TEMP); ++counter; - mutex_unlock_static (L_CREATE_TEMP); { uint8_t rndbytes[16]; diff --git a/multi.c b/multi.c index 92ab4ca3..c2f2dbcd 100644 --- a/multi.c +++ b/multi.c @@ -633,7 +633,6 @@ multi_create_instance (struct multi_context *m, const struct mroute_addr *real) ALLOC_OBJ_CLEAR (mi, struct multi_instance); - mutex_init (&mi->mutex); mi->gc = gc_new (); multi_instance_inc_refcount (mi); mi->vaddr_handle = -1; diff --git a/multi.h b/multi.h index ad26c127..585d0d50 100644 --- a/multi.h +++ b/multi.h @@ -56,7 +56,6 @@ struct multi_reap struct multi_instance { struct schedule_entry se; /* this must be the first element of the structure */ struct gc_arena gc; - MUTEX_DEFINE (mutex); bool defined; bool halt; int refcount; @@ -274,7 +273,6 @@ multi_instance_dec_refcount (struct multi_instance *mi) if (--mi->refcount <= 0) { gc_free (&mi->gc); - mutex_destroy (&mi->mutex); free (mi); } } diff --git a/options.c b/options.c index 8177732d..f0b93100 100644 --- a/options.c +++ b/options.c @@ -69,9 +69,6 @@ const char title_string[] = #ifdef PRODUCT_TAP_DEBUG " [TAPDBG]" #endif -#ifdef USE_PTHREAD - " [PTHREAD]" -#endif #ifdef ENABLE_PKCS11 " [PKCS11]" #endif @@ -288,13 +285,6 @@ static const char usage_message[] = "--suppress-timestamps : Don't log timestamps to stdout/stderr.\n" "--writepid file : Write main process ID to file.\n" "--nice n : Change process priority (>0 = lower, <0 = higher).\n" -#if 0 -#ifdef USE_PTHREAD - "--nice-work n : Change thread priority of work thread. The work\n" - " thread is used for background processing such as\n" - " RSA key number crunching.\n" -#endif -#endif "--echo [parms ...] : Echo parameters to log output.\n" "--verb n : Set output verbosity to n (default=%d):\n" " (Level 3 is recommended if you want a good summary\n" @@ -725,9 +715,6 @@ init_options (struct options *o, const bool init_gc) o->tuntap_options.dhcp_masq_offset = 0; /* use network address as internal DHCP server address */ o->route_method = ROUTE_METHOD_ADAPTIVE; #endif -#ifdef USE_PTHREAD - o->n_threads = 1; -#endif #if P2MP_SERVER o->real_hash_size = 256; o->virtual_hash_size = 256; @@ -875,9 +862,6 @@ is_persist_option (const struct options *o) || o->persist_key || o->persist_local_ip || o->persist_remote_ip -#ifdef USE_PTHREAD - || o->n_threads >= 2 -#endif ; } @@ -4033,26 +4017,6 @@ add_option (struct options *options, goto err; #endif } -#ifdef USE_PTHREAD - else if (streq (p[0], "nice-work") && p[1]) - { - VERIFY_PERMISSION (OPT_P_NICE); - options->nice_work = atoi (p[1]); - } - else if (streq (p[0], "threads") && p[1]) - { - int n_threads; - - VERIFY_PERMISSION (OPT_P_GENERAL); - n_threads = positive_atoi (p[1]); - if (n_threads < 1) - { - msg (msglevel, "--threads parameter must be at least 1"); - goto err; - } - options->n_threads = n_threads; - } -#endif else if (streq (p[0], "shaper") && p[1]) { #ifdef HAVE_GETTIMEOFDAY diff --git a/options.h b/options.h index 9a331d81..c1c96cd2 100644 --- a/options.h +++ b/options.h @@ -328,11 +328,6 @@ struct options struct plugin_option_list *plugin_list; #endif -#ifdef USE_PTHREAD - int n_threads; - int nice_work; -#endif - #if P2MP #if P2MP_SERVER diff --git a/otime.c b/otime.c index cba953d8..5b553220 100644 --- a/otime.c +++ b/otime.c @@ -123,10 +123,8 @@ time_string (time_t t, int usec, bool show_usec, struct gc_arena *gc) } } - mutex_lock_static (L_CTIME); t = tv.tv_sec; buf_printf (&out, "%s", ctime(&t)); - mutex_unlock_static (L_CTIME); buf_rmtail (&out, '\n'); if (show_usec && tv.tv_usec) diff --git a/otime.h b/otime.h index 3c069226..8e8aaaf7 100644 --- a/otime.h +++ b/otime.h @@ -28,7 +28,6 @@ #include "common.h" #include "integer.h" #include "buffer.h" -#include "thread.h" struct frequency_limit { diff --git a/perf.c b/perf.c index 9af680fa..a149c07d 100644 --- a/perf.c +++ b/perf.c @@ -33,10 +33,6 @@ #include "memdbg.h" -#ifdef USE_PTHREAD -#error ENABLE_PERFORMANCE_METRICS is incompatible with USE_PTHREAD -#endif - static const char *metric_names[] = { "PERF_BIO_READ_PLAINTEXT", "PERF_BIO_WRITE_PLAINTEXT", diff --git a/plugin.c b/plugin.c index d04c01d3..0e7779e7 100644 --- a/plugin.c +++ b/plugin.c @@ -558,8 +558,6 @@ plugin_call (const struct plugin_list *pl, bool error = false; bool deferred = false; - mutex_lock_static (L_PLUGIN); - setenv_del (es, "script_type"); envp = make_env_array (es, false, &gc); @@ -588,8 +586,6 @@ plugin_call (const struct plugin_list *pl, if (pr) pr->n = i; - mutex_unlock_static (L_PLUGIN); - gc_free (&gc); if (type == OPENVPN_PLUGIN_ENABLE_PF && success) diff --git a/pool.h b/pool.h index 10cf670c..d2a7ceb1 100644 --- a/pool.h +++ b/pool.h @@ -31,7 +31,6 @@ #include "basic.h" #include "status.h" -#include "thread.h" #define IFCONFIG_POOL_MAX 65536 #define IFCONFIG_POOL_MIN_NETBITS 16 diff --git a/schedule.c b/schedule.c index 724613e4..e1b7a60a 100644 --- a/schedule.c +++ b/schedule.c @@ -363,24 +363,20 @@ schedule_init (void) struct schedule *s; ALLOC_OBJ_CLEAR (s, struct schedule); - mutex_init (&s->mutex); return s; } void schedule_free (struct schedule *s) { - mutex_destroy (&s->mutex); free (s); } void schedule_remove_entry (struct schedule *s, struct schedule_entry *e) { - mutex_lock (&s->mutex); s->earliest_wakeup = NULL; /* invalidate cache */ schedule_remove_node (s, e); - mutex_unlock (&s->mutex); } /* diff --git a/schedule.h b/schedule.h index e2a5e0dc..d881c384 100644 --- a/schedule.h +++ b/schedule.h @@ -42,7 +42,6 @@ /*#define SCHEDULE_TEST*/ #include "otime.h" -#include "thread.h" #include "error.h" struct schedule_entry @@ -56,7 +55,6 @@ struct schedule_entry struct schedule { - MUTEX_DEFINE (mutex); struct schedule_entry *earliest_wakeup; /* cached earliest wakeup */ struct schedule_entry *root; /* the root of the treap (btree) */ }; @@ -100,14 +98,12 @@ schedule_add_entry (struct schedule *s, const struct timeval *tv, unsigned int sigma) { - mutex_lock (&s->mutex); if (!IN_TREE (e) || !sigma || !tv_within_sigma (tv, &e->tv, sigma)) { e->tv = *tv; schedule_add_modify (s, e); s->earliest_wakeup = NULL; /* invalidate cache */ } - mutex_unlock (&s->mutex); } /* @@ -122,8 +118,6 @@ schedule_get_earliest_wakeup (struct schedule *s, { struct schedule_entry *ret; - mutex_lock (&s->mutex); - /* cache result */ if (!s->earliest_wakeup) s->earliest_wakeup = schedule_find_least (s->root); @@ -131,8 +125,6 @@ schedule_get_earliest_wakeup (struct schedule *s, if (ret) *wakeup = ret->tv; - mutex_unlock (&s->mutex); - return ret; } diff --git a/socket.c b/socket.c index 7d20bb0f..d330558f 100644 --- a/socket.c +++ b/socket.c @@ -26,7 +26,6 @@ #include "socket.h" #include "fdmisc.h" -#include "thread.h" #include "misc.h" #include "gremlin.h" #include "plugin.h" @@ -1933,10 +1932,8 @@ print_sockaddr_ex (const struct openvpn_sockaddr *addr, struct buffer out = alloc_buf_gc (64, gc); const int port = ntohs (addr->sa.sin_port); - mutex_lock_static (L_INET_NTOA); if (!(flags & PS_DONT_SHOW_ADDR)) buf_printf (&out, "%s", (addr_defined (addr) ? inet_ntoa (addr->sa.sin_addr) : "[undef]")); - mutex_unlock_static (L_INET_NTOA); if (((flags & PS_SHOW_PORT) || (addr_defined (addr) && (flags & PS_SHOW_PORT_IF_DEFINED))) && port) @@ -1998,9 +1995,7 @@ print_in_addr_t (in_addr_t addr, unsigned int flags, struct gc_arena *gc) CLEAR (ia); ia.s_addr = (flags & IA_NET_ORDER) ? addr : htonl (addr); - mutex_lock_static (L_INET_NTOA); buf_printf (&out, "%s", inet_ntoa (ia)); - mutex_unlock_static (L_INET_NTOA); } return BSTR (&out); } @@ -2016,9 +2011,7 @@ setenv_sockaddr (struct env_set *es, const char *name_prefix, const struct openv else openvpn_snprintf (name_buf, sizeof (name_buf), "%s", name_prefix); - mutex_lock_static (L_INET_NTOA); setenv_str (es, name_buf, inet_ntoa (addr->sa.sin_addr)); - mutex_unlock_static (L_INET_NTOA); if ((flags & SA_IP_PORT) && addr->sa.sin_port) { diff --git a/ssl.c b/ssl.c index 9780a67d..5809f945 100644 --- a/ssl.c +++ b/ssl.c @@ -39,7 +39,6 @@ #include "common.h" #include "integer.h" #include "socket.h" -#include "thread.h" #include "misc.h" #include "fdmisc.h" #include "interval.h" diff --git a/ssl.h b/ssl.h index 93aac783..709b56a2 100644 --- a/ssl.h +++ b/ssl.h @@ -42,7 +42,6 @@ #include "reliable.h" #include "socket.h" #include "mtu.h" -#include "thread.h" #include "options.h" #include "plugin.h" diff --git a/syshead.h b/syshead.h index 6e81103b..3b8ab89e 100644 --- a/syshead.h +++ b/syshead.h @@ -537,24 +537,6 @@ socket_defined (const socket_descriptor_t sd) #define ENABLE_BUFFER_LIST #endif -/* - * Do we have pthread capability? - */ -#ifdef USE_PTHREAD -#if defined(USE_CRYPTO) && defined(USE_SSL) && P2MP -#include -#else -#undef USE_PTHREAD -#endif -#endif - -/* - * Pthread support is currently experimental (and quite unfinished). - */ -#if 1 /* JYFIXME -- if defined, disable pthread */ -#undef USE_PTHREAD -#endif - /* * Should we include OCC (options consistency check) code? */ diff --git a/thread.c b/thread.c deleted file mode 100644 index fef7db69..00000000 --- a/thread.c +++ /dev/null @@ -1,156 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2002-2009 OpenVPN Technologies, Inc. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program (see the file COPYING included with this - * distribution); if not, write to the Free Software Foundation, Inc., - * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ - -#include "syshead.h" - -#ifdef USE_PTHREAD - -#include "thread.h" -#include "buffer.h" -#include "common.h" -#include "error.h" -#include "crypto.h" - -#include "memdbg.h" - -static struct sparse_mutex *ssl_mutex; /* GLOBAL */ - -static void -ssl_pthreads_locking_callback (int mode, int type, char *file, int line) -{ - dmsg (D_OPENSSL_LOCK, "SSL LOCK thread=%4lu mode=%s lock=%s %s:%d", - CRYPTO_thread_id (), - (mode & CRYPTO_LOCK) ? "l" : "u", - (type & CRYPTO_READ) ? "r" : "w", file, line); - - if (mode & CRYPTO_LOCK) - pthread_mutex_lock (&ssl_mutex[type].mutex); - else - pthread_mutex_unlock (&ssl_mutex[type].mutex); -} - -static unsigned long -ssl_pthreads_thread_id (void) -{ - unsigned long ret; - - ret = (unsigned long) pthread_self (); - return ret; -} - -static void -ssl_thread_setup (void) -{ - int i; - -#error L_MSG needs to be initialized as a recursive mutex - - ssl_mutex = OPENSSL_malloc (CRYPTO_num_locks () * sizeof (struct sparse_mutex)); - for (i = 0; i < CRYPTO_num_locks (); i++) - pthread_mutex_init (&ssl_mutex[i].mutex, NULL); - - CRYPTO_set_id_callback ((unsigned long (*)(void)) ssl_pthreads_thread_id); - CRYPTO_set_locking_callback ((void (*)(int, int, const char*, int)) ssl_pthreads_locking_callback); -} - -static void -ssl_thread_cleanup (void) -{ - int i; - - dmsg (D_OPENSSL_LOCK, "SSL LOCK cleanup"); - CRYPTO_set_locking_callback (NULL); - for (i = 0; i < CRYPTO_num_locks (); i++) - pthread_mutex_destroy (&ssl_mutex[i].mutex); - OPENSSL_free (ssl_mutex); -} - -struct sparse_mutex mutex_array[N_MUTEXES]; /* GLOBAL */ -bool pthread_initialized; /* GLOBAL */ - -openvpn_thread_t -openvpn_thread_create (void *(*start_routine) (void *), void* arg) -{ - openvpn_thread_t ret; - ASSERT (pthread_initialized); - ASSERT (!pthread_create (&ret, NULL, start_routine, arg)); - dmsg (D_THREAD_DEBUG, "CREATE THREAD ID=%lu", (unsigned long)ret); - return ret; -} - -void -openvpn_thread_join (openvpn_thread_t id) -{ - ASSERT (pthread_initialized); - pthread_join (id, NULL); -} - -void -openvpn_thread_init () -{ - int i; - - ASSERT (!pthread_initialized); - - msg (M_INFO, "PTHREAD support initialized"); - - /* initialize OpenSSL library locking */ -#if defined(USE_CRYPTO) && defined(USE_SSL) - ssl_thread_setup(); -#endif - - /* initialize static mutexes */ - for (i = 0; i < N_MUTEXES; i++) - ASSERT (!pthread_mutex_init (&mutex_array[i].mutex, NULL)); - - msg_thread_init (); - - pthread_initialized = true; -} - -void -openvpn_thread_cleanup () -{ - if (pthread_initialized) - { - int i; - - pthread_initialized = false; - - /* cleanup OpenSSL library locking */ -#if defined(USE_CRYPTO) && defined(USE_SSL) - ssl_thread_cleanup(); -#endif - - /* destroy static mutexes */ - for (i = 0; i < N_MUTEXES; i++) - ASSERT (!pthread_mutex_destroy (&mutex_array[i].mutex)); - - msg_thread_uninit (); - } -} - -#else -static void dummy(void) {} -#endif diff --git a/thread.h b/thread.h deleted file mode 100644 index eeef3228..00000000 --- a/thread.h +++ /dev/null @@ -1,235 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2002-2009 OpenVPN Technologies, Inc. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program (see the file COPYING included with this - * distribution); if not, write to the Free Software Foundation, Inc., - * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ - -#ifndef THREAD_H -#define THREAD_H - -#include "basic.h" -#include "common.h" - -/* - * OpenVPN static mutex locks, by mutex type - */ -#define L_UNUSED 0 -#define L_CTIME 1 -#define L_INET_NTOA 2 -#define L_MSG 3 -#define L_STRERR 4 -#define L_PUTENV 5 -#define L_PRNG 6 -#define L_GETTIMEOFDAY 7 -#define L_ENV_SET 8 -#define L_SYSTEM 9 -#define L_CREATE_TEMP 10 -#define L_PLUGIN 11 -#define N_MUTEXES 12 - -#ifdef USE_PTHREAD - -#define MAX_THREADS 50 - -#define CACHE_LINE_SIZE 128 - -/* - * Improve SMP performance by making sure that each - * mutex resides in its own cache line. - */ -struct sparse_mutex -{ - pthread_mutex_t mutex; - uint8_t dummy [CACHE_LINE_SIZE - sizeof (pthread_mutex_t)]; -}; - -typedef pthread_t openvpn_thread_t; - -extern bool pthread_initialized; - -extern struct sparse_mutex mutex_array[N_MUTEXES]; - -#define MUTEX_DEFINE(lock) pthread_mutex_t lock -#define MUTEX_PTR_DEFINE(lock) pthread_mutex_t *lock - -static inline bool -openvpn_thread_enabled (void) -{ - return pthread_initialized; -} - -static inline openvpn_thread_t -openvpn_thread_self (void) -{ - return pthread_initialized ? pthread_self() : 0; -} - -static inline void -mutex_init (pthread_mutex_t *mutex) -{ - if (mutex) - pthread_mutex_init (mutex, NULL); -} - -static inline void -mutex_destroy (pthread_mutex_t *mutex) -{ - if (mutex) - pthread_mutex_destroy (mutex); -} - -static inline void -mutex_lock (pthread_mutex_t *mutex) -{ - if (pthread_initialized && mutex) - pthread_mutex_lock (mutex); -} - -static inline bool -mutex_trylock (pthread_mutex_t *mutex) -{ - if (pthread_initialized && mutex) - return pthread_mutex_trylock (mutex) == 0; - else - return true; -} - -static inline void -mutex_unlock (pthread_mutex_t *mutex) -{ - if (pthread_initialized && mutex) - { - pthread_mutex_unlock (mutex); -#if 1 /* JYFIXME: if race conditions exist, make them more likely to occur */ - sleep (0); -#endif - } -} - -static inline void -mutex_cycle (pthread_mutex_t *mutex) -{ - if (pthread_initialized && mutex) - { - pthread_mutex_unlock (mutex); - sleep (0); - pthread_mutex_lock (mutex); - } -} - -static inline void -mutex_lock_static (int type) -{ - mutex_lock (&mutex_array[type].mutex); -} - -static inline void -mutex_unlock_static (int type) -{ - mutex_unlock (&mutex_array[type].mutex); -} - -static inline void -mutex_cycle_static (int type) -{ - mutex_cycle (&mutex_array[type].mutex); -} - -void openvpn_thread_init (void); -void openvpn_thread_cleanup (void); - -openvpn_thread_t openvpn_thread_create (void *(*start_routine) (void *), void* arg); -void openvpn_thread_join (openvpn_thread_t id); - -#else /* USE_PTHREAD */ - -typedef int openvpn_thread_t; - -#if defined(_MSC_VER) || PEDANTIC - -#define MUTEX_DEFINE(lock) int eat_semicolon -#define MUTEX_PTR_DEFINE(lock) int eat_semicolon - -#else - -#define MUTEX_DEFINE(lock) -#define MUTEX_PTR_DEFINE(lock) - -#endif - -#define mutex_init(m) -#define mutex_destroy(m) -#define mutex_lock(m) -#define mutex_trylock(m) (true) -#define mutex_unlock(m) -#define mutex_cycle(m) - -static inline bool -openvpn_thread_enabled (void) -{ - return false; -} - -static inline openvpn_thread_t -openvpn_thread_self (void) -{ - return 0; -} - -static inline void -openvpn_thread_init (void) -{ -} - -static inline void -openvpn_thread_cleanup (void) -{ -} - -static inline openvpn_thread_t -openvpn_thread_create (void *(*start_routine) (void *), void* arg) -{ - return 0; -} - -static inline void -work_thread_join (openvpn_thread_t id) -{ -} - -static inline void -mutex_lock_static (int type) -{ -} - -static inline void -mutex_unlock_static (int type) -{ -} - -static inline void -mutex_cycle_static (int type) -{ -} - -#endif /* USE_PTHREAD */ - -#endif /* THREAD_H */ From cc88a2695f4a54e27143efeae62de24fec8e26a1 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Sat, 28 Aug 2010 20:44:07 +0200 Subject: [PATCH 2/9] Clean-up: Remove more dead and inactive code paths These code paths was practically not needed with no locking mechanisms enabled and was just bloating the source code. Signed-off-by: David Sommerseth Acked-by: James Yonan --- buffer.c | 4 ---- forward.c | 10 ---------- mbuf.c | 4 ++-- mbuf.h | 2 +- mroute.c | 6 ------ mroute.h | 13 ------------- mtcp.c | 2 +- multi.c | 6 +----- openvpn.h | 3 --- ssl.c | 4 ---- ssl.h | 3 --- 11 files changed, 5 insertions(+), 52 deletions(-) diff --git a/buffer.c b/buffer.c index e9ae21a1..729495cf 100644 --- a/buffer.c +++ b/buffer.c @@ -298,10 +298,8 @@ gc_malloc (size_t size, bool clear, struct gc_arena *a) #endif check_malloc_return (e); ret = (char *) e + sizeof (struct gc_entry); - /*mutex_lock_static (L_GC_MALLOC);*/ e->next = a->list; a->list = e; - /*mutex_unlock_static (L_GC_MALLOC);*/ } else { @@ -323,10 +321,8 @@ void x_gc_free (struct gc_arena *a) { struct gc_entry *e; - /*mutex_lock_static (L_GC_MALLOC);*/ e = a->list; a->list = NULL; - /*mutex_unlock_static (L_GC_MALLOC);*/ while (e != NULL) { diff --git a/forward.c b/forward.c index d563e11a..463a2e81 100644 --- a/forward.c +++ b/forward.c @@ -454,7 +454,6 @@ encrypt_sign (struct context *c, bool comp_frag) */ if (c->c2.tls_multi) { - /*tls_mutex_lock (c->c2.tls_multi);*/ tls_pre_encrypt (c->c2.tls_multi, &c->c2.buf, &c->c2.crypto_options); } #endif @@ -482,7 +481,6 @@ encrypt_sign (struct context *c, bool comp_frag) if (c->c2.tls_multi) { tls_post_encrypt (c->c2.tls_multi, &c->c2.buf); - /*tls_mutex_unlock (c->c2.tls_multi);*/ } #endif #endif @@ -790,7 +788,6 @@ process_incoming_link (struct context *c) * will load crypto_options with the correct encryption key * and return false. */ - /*tls_mutex_lock (c->c2.tls_multi);*/ if (tls_pre_decrypt (c->c2.tls_multi, &c->c2.from, &c->c2.buf, &c->c2.crypto_options)) { interval_action (&c->c2.tmp_int); @@ -813,13 +810,6 @@ process_incoming_link (struct context *c) /* authenticate and decrypt the incoming packet */ decrypt_status = openvpn_decrypt (&c->c2.buf, c->c2.buffers->decrypt_buf, &c->c2.crypto_options, &c->c2.frame); -#ifdef USE_SSL - if (c->c2.tls_multi) - { - /*tls_mutex_unlock (c->c2.tls_multi);*/ - } -#endif - if (!decrypt_status && link_socket_connection_oriented (c->c2.link_socket)) { /* decryption errors are fatal in TCP mode */ diff --git a/mbuf.c b/mbuf.c index 8ffda00c..cb83ac70 100644 --- a/mbuf.c +++ b/mbuf.c @@ -90,7 +90,7 @@ mbuf_add_item (struct mbuf_set *ms, const struct mbuf_item *item) if (ms->len == ms->capacity) { struct mbuf_item rm; - ASSERT (mbuf_extract_item (ms, &rm, false)); + ASSERT (mbuf_extract_item (ms, &rm)); mbuf_free_buf (rm.buffer); msg (D_MULTI_DROPPED, "MBUF: mbuf packet dropped"); } @@ -104,7 +104,7 @@ mbuf_add_item (struct mbuf_set *ms, const struct mbuf_item *item) } bool -mbuf_extract_item (struct mbuf_set *ms, struct mbuf_item *item, const bool lock) +mbuf_extract_item (struct mbuf_set *ms, struct mbuf_item *item) { bool ret = false; if (ms) diff --git a/mbuf.h b/mbuf.h index 7be45559..230b5b5a 100644 --- a/mbuf.h +++ b/mbuf.h @@ -73,7 +73,7 @@ void mbuf_free_buf (struct mbuf_buffer *mb); void mbuf_add_item (struct mbuf_set *ms, const struct mbuf_item *item); -bool mbuf_extract_item (struct mbuf_set *ms, struct mbuf_item *item, const bool lock); +bool mbuf_extract_item (struct mbuf_set *ms, struct mbuf_item *item); void mbuf_dereference_instance (struct mbuf_set *ms, struct multi_instance *mi); diff --git a/mroute.c b/mroute.c index 9d8fa664..f068e063 100644 --- a/mroute.c +++ b/mroute.c @@ -360,7 +360,6 @@ mroute_helper_init (int ageable_ttl_secs) { struct mroute_helper *mh; ALLOC_OBJ_CLEAR (mh, struct mroute_helper); - /*mutex_init (&mh->mutex);*/ mh->ageable_ttl_secs = ageable_ttl_secs; return mh; } @@ -398,12 +397,10 @@ mroute_helper_add_iroute (struct mroute_helper *mh, const struct iroute *ir) if (ir->netbits >= 0) { ASSERT (ir->netbits < MR_HELPER_NET_LEN); - mroute_helper_lock (mh); ++mh->cache_generation; ++mh->net_len_refcount[ir->netbits]; if (mh->net_len_refcount[ir->netbits] == 1) mroute_helper_regenerate (mh); - mroute_helper_unlock (mh); } } @@ -413,20 +410,17 @@ mroute_helper_del_iroute (struct mroute_helper *mh, const struct iroute *ir) if (ir->netbits >= 0) { ASSERT (ir->netbits < MR_HELPER_NET_LEN); - mroute_helper_lock (mh); ++mh->cache_generation; --mh->net_len_refcount[ir->netbits]; ASSERT (mh->net_len_refcount[ir->netbits] >= 0); if (!mh->net_len_refcount[ir->netbits]) mroute_helper_regenerate (mh); - mroute_helper_unlock (mh); } } void mroute_helper_free (struct mroute_helper *mh) { - /*mutex_destroy (&mh->mutex);*/ free (mh); } diff --git a/mroute.h b/mroute.h index 9b9087dd..283d4c28 100644 --- a/mroute.h +++ b/mroute.h @@ -91,7 +91,6 @@ struct mroute_addr { * Used to help maintain CIDR routing table. */ struct mroute_helper { - /*MUTEX_DEFINE (mutex);*/ unsigned int cache_generation; /* incremented when route added */ int ageable_ttl_secs; /* host route cache entry time-to-live*/ int n_net_len; /* length of net_len array */ @@ -159,18 +158,6 @@ mroute_extract_addr_from_packet (struct mroute_addr *src, return ret; } -static inline void -mroute_helper_lock (struct mroute_helper *mh) -{ - /*mutex_lock (&mh->mutex);*/ -} - -static inline void -mroute_helper_unlock (struct mroute_helper *mh) -{ - /*mutex_unlock (&mh->mutex);*/ -} - static inline bool mroute_addr_equal (const struct mroute_addr *a1, const struct mroute_addr *a2) { diff --git a/mtcp.c b/mtcp.c index 72c9618e..cc0bab87 100644 --- a/mtcp.c +++ b/mtcp.c @@ -264,7 +264,7 @@ multi_tcp_process_outgoing_link_ready (struct multi_context *m, struct multi_ins ASSERT (mi); /* extract from queue */ - if (mbuf_extract_item (mi->tcp_link_out_deferred, &item, true)) /* ciphertext IP packet */ + if (mbuf_extract_item (mi->tcp_link_out_deferred, &item)) /* ciphertext IP packet */ { dmsg (D_MULTI_TCP, "MULTI TCP: transmitting previously deferred packet"); diff --git a/multi.c b/multi.c index c2f2dbcd..bf9e8896 100644 --- a/multi.c +++ b/multi.c @@ -1000,8 +1000,6 @@ multi_get_instance_by_virtual_addr (struct multi_context *m, struct mroute_addr tryaddr; int i; - mroute_helper_lock (rh); - /* cycle through each CIDR length */ for (i = 0; i < rh->n_net_len; ++i) { @@ -1022,8 +1020,6 @@ multi_get_instance_by_virtual_addr (struct multi_context *m, break; } } - - mroute_helper_unlock (rh); } #ifdef ENABLE_DEBUG @@ -2240,7 +2236,7 @@ multi_get_queue (struct mbuf_set *ms) { struct mbuf_item item; - if (mbuf_extract_item (ms, &item, true)) /* cleartext IP packet */ + if (mbuf_extract_item (ms, &item)) /* cleartext IP packet */ { unsigned int pipv4_flags = PIPV4_PASSTOS; diff --git a/openvpn.h b/openvpn.h index 1df46a1a..6e6e1f89 100644 --- a/openvpn.h +++ b/openvpn.h @@ -460,9 +460,6 @@ struct context /* true on initial VPN iteration */ bool first_time; - /* used by multi-client code to lock the context */ - /*MUTEX_DEFINE (mutex);*/ - /* context modes */ # define CM_P2P 0 /* standalone point-to-point session or client */ # define CM_TOP 1 /* top level of a multi-client or point-to-multipoint server */ diff --git a/ssl.c b/ssl.c index 5809f945..c05d34fc 100644 --- a/ssl.c +++ b/ssl.c @@ -3814,8 +3814,6 @@ tls_process (struct tls_multi *multi, msg (D_TLS_DEBUG_LOW, "TLS: tls_process: killed expiring key"); } - /*mutex_cycle (multi->mutex);*/ - do { update_time (); @@ -4099,7 +4097,6 @@ tls_process (struct tls_multi *multi, } } } - /*mutex_cycle (multi->mutex);*/ } while (state_change); @@ -4253,7 +4250,6 @@ tls_multi_process (struct tls_multi *multi, reset_session (multi, session); } } - /*mutex_cycle (multi->mutex);*/ } update_time (); diff --git a/ssl.h b/ssl.h index 709b56a2..78ce57ec 100644 --- a/ssl.h +++ b/ssl.h @@ -573,9 +573,6 @@ struct tls_session */ struct tls_multi { - /* used to coordinate access between main thread and TLS thread */ - /*MUTEX_PTR_DEFINE (mutex);*/ - /* const options and config info */ struct tls_options opt; From 6af422162fbc1c505526157ecf630e37694dbc7b Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Sat, 28 Aug 2010 20:52:19 +0200 Subject: [PATCH 3/9] Clean-up: Removing useless code - hash related functions Removed even more function which where practically empty and took away some function arguments which were not used. Signed-off-by: David Sommerseth Acked-by: James Yonan --- list.c | 11 ++++------- list.h | 25 ++++--------------------- mtcp.c | 3 --- mudp.c | 3 --- multi.c | 25 +++++++++++-------------- pf.c | 2 +- 6 files changed, 20 insertions(+), 49 deletions(-) diff --git a/list.c b/list.c index d1e5a157..d4d4ea80 100644 --- a/list.c +++ b/list.c @@ -165,12 +165,12 @@ hash_add (struct hash *hash, const void *key, void *value, bool replace) } void -hash_remove_by_value (struct hash *hash, void *value, bool autolock) +hash_remove_by_value (struct hash *hash, void *value) { struct hash_iterator hi; struct hash_element *he; - hash_iterator_init (hash, &hi, autolock); + hash_iterator_init (hash, &hi); while ((he = hash_iterator_next (&hi))) { if (he->value == value) @@ -221,7 +221,6 @@ void_ptr_compare_function (const void *key1, const void *key2) void hash_iterator_init_range (struct hash *hash, struct hash_iterator *hi, - bool autolock, int start_bucket, int end_bucket) { @@ -233,7 +232,6 @@ hash_iterator_init_range (struct hash *hash, hi->hash = hash; hi->elem = NULL; hi->bucket = NULL; - hi->autolock = autolock; hi->last = NULL; hi->bucket_marked = false; hi->bucket_index_start = start_bucket; @@ -243,10 +241,9 @@ hash_iterator_init_range (struct hash *hash, void hash_iterator_init (struct hash *hash, - struct hash_iterator *hi, - bool autolock) + struct hash_iterator *hi) { - hash_iterator_init_range (hash, hi, autolock, 0, hash->n_buckets); + hash_iterator_init_range (hash, hi, 0, hash->n_buckets); } static inline void diff --git a/list.h b/list.h index 138e894d..ab5ea093 100644 --- a/list.h +++ b/list.h @@ -88,7 +88,7 @@ bool hash_remove_fast (struct hash *hash, const void *key, uint32_t hv); -void hash_remove_by_value (struct hash *hash, void *value, bool autolock); +void hash_remove_by_value (struct hash *hash, void *value); struct hash_iterator { @@ -98,18 +98,16 @@ struct hash_iterator struct hash_element *elem; struct hash_element *last; bool bucket_marked; - bool autolock; int bucket_index_start; int bucket_index_end; }; void hash_iterator_init_range (struct hash *hash, struct hash_iterator *hi, - bool autolock, int start_bucket, int end_bucket); -void hash_iterator_init (struct hash *hash, struct hash_iterator *iter, bool autolock); +void hash_iterator_init (struct hash *hash, struct hash_iterator *iter); struct hash_element *hash_iterator_next (struct hash_iterator *hi); void hash_iterator_delete_element (struct hash_iterator *hi); void hash_iterator_free (struct hash_iterator *hi); @@ -147,21 +145,12 @@ hash_bucket (struct hash *hash, uint32_t hv) return &hash->buckets[hv & hash->mask]; } -static inline void -hash_bucket_lock (struct hash_bucket *bucket) -{ -} - -static inline void -hash_bucket_unlock (struct hash_bucket *bucket) -{ -} - static inline void * -hash_lookup_lock (struct hash *hash, const void *key, uint32_t hv) +hash_lookup (struct hash *hash, const void *key) { void *ret = NULL; struct hash_element *he; + uint32_t hv = hash_value (hash, key); struct hash_bucket *bucket = &hash->buckets[hv & hash->mask]; he = hash_lookup_fast (hash, bucket, key, hv); @@ -171,12 +160,6 @@ hash_lookup_lock (struct hash *hash, const void *key, uint32_t hv) return ret; } -static inline void * -hash_lookup (struct hash *hash, const void *key) -{ - return hash_lookup_lock (hash, key, hash_value (hash, key)); -} - /* NOTE: assumes that key is not a duplicate */ static inline void hash_add_fast (struct hash *hash, diff --git a/mtcp.c b/mtcp.c index cc0bab87..3d68a9c5 100644 --- a/mtcp.c +++ b/mtcp.c @@ -112,7 +112,6 @@ multi_create_instance_tcp (struct multi_context *m) const uint32_t hv = hash_value (hash, &mi->real); struct hash_bucket *bucket = hash_bucket (hash, hv); - hash_bucket_lock (bucket); he = hash_lookup_fast (hash, bucket, &mi->real, hv); if (he) @@ -128,8 +127,6 @@ multi_create_instance_tcp (struct multi_context *m) hash_add_fast (hash, bucket, &mi->real, hv, mi); mi->did_real_hash = true; - - hash_bucket_unlock (bucket); } #ifdef ENABLE_DEBUG diff --git a/mudp.c b/mudp.c index 815ed1c8..77468b21 100644 --- a/mudp.c +++ b/mudp.c @@ -51,7 +51,6 @@ multi_get_create_instance_udp (struct multi_context *m) const uint32_t hv = hash_value (hash, &real); struct hash_bucket *bucket = hash_bucket (hash, hv); - hash_bucket_lock (bucket); he = hash_lookup_fast (hash, bucket, &real, hv); if (he) @@ -81,8 +80,6 @@ multi_get_create_instance_udp (struct multi_context *m) } } - hash_bucket_unlock (bucket); - #ifdef ENABLE_DEBUG if (check_debug_level (D_MULTI_DEBUG)) { diff --git a/multi.c b/multi.c index bf9e8896..4cd7468d 100644 --- a/multi.c +++ b/multi.c @@ -146,7 +146,7 @@ multi_reap_range (const struct multi_context *m, } dmsg (D_MULTI_DEBUG, "MULTI: REAP range %d -> %d", start_bucket, end_bucket); - hash_iterator_init_range (m->vhash, &hi, true, start_bucket, end_bucket); + hash_iterator_init_range (m->vhash, &hi, start_bucket, end_bucket); while ((he = hash_iterator_next (&hi)) != NULL) { struct multi_route *r = (struct multi_route *) he->value; @@ -587,7 +587,7 @@ multi_uninit (struct multi_context *m) struct hash_iterator hi; struct hash_element *he; - hash_iterator_init (m->iter, &hi, true); + hash_iterator_init (m->iter, &hi); while ((he = hash_iterator_next (&hi))) { struct multi_instance *mi = (struct multi_instance *) he->value; @@ -723,7 +723,7 @@ multi_print_status (struct multi_context *m, struct status_output *so, const int status_printf (so, PACKAGE_NAME " CLIENT LIST"); status_printf (so, "Updated,%s", time_string (0, 0, false, &gc_top)); status_printf (so, "Common Name,Real Address,Bytes Received,Bytes Sent,Connected Since"); - hash_iterator_init (m->hash, &hi, true); + hash_iterator_init (m->hash, &hi); while ((he = hash_iterator_next (&hi))) { struct gc_arena gc = gc_new (); @@ -744,7 +744,7 @@ multi_print_status (struct multi_context *m, struct status_output *so, const int status_printf (so, "ROUTING TABLE"); status_printf (so, "Virtual Address,Common Name,Real Address,Last Ref"); - hash_iterator_init (m->vhash, &hi, true); + hash_iterator_init (m->vhash, &hi); while ((he = hash_iterator_next (&hi))) { struct gc_arena gc = gc_new (); @@ -787,7 +787,7 @@ multi_print_status (struct multi_context *m, struct status_output *so, const int status_printf (so, "TIME%c%s%c%u", sep, time_string (now, 0, false, &gc_top), sep, (unsigned int)now); status_printf (so, "HEADER%cCLIENT_LIST%cCommon Name%cReal Address%cVirtual Address%cBytes Received%cBytes Sent%cConnected Since%cConnected Since (time_t)", sep, sep, sep, sep, sep, sep, sep, sep); - hash_iterator_init (m->hash, &hi, true); + hash_iterator_init (m->hash, &hi); while ((he = hash_iterator_next (&hi))) { struct gc_arena gc = gc_new (); @@ -810,7 +810,7 @@ multi_print_status (struct multi_context *m, struct status_output *so, const int status_printf (so, "HEADER%cROUTING_TABLE%cVirtual Address%cCommon Name%cReal Address%cLast Ref%cLast Ref (time_t)", sep, sep, sep, sep, sep, sep); - hash_iterator_init (m->vhash, &hi, true); + hash_iterator_init (m->vhash, &hi); while ((he = hash_iterator_next (&hi))) { struct gc_arena gc = gc_new (); @@ -849,7 +849,7 @@ multi_print_status (struct multi_context *m, struct status_output *so, const int #ifdef PACKET_TRUNCATION_CHECK { status_printf (so, "HEADER,ERRORS,Common Name,TUN Read Trunc,TUN Write Trunc,Pre-encrypt Trunc,Post-decrypt Trunc"); - hash_iterator_init (m->hash, &hi, true); + hash_iterator_init (m->hash, &hi); while ((he = hash_iterator_next (&hi))) { struct gc_arena gc = gc_new (); @@ -895,8 +895,6 @@ multi_learn_addr (struct multi_context *m, struct multi_route *oldroute = NULL; struct multi_instance *owner = NULL; - hash_bucket_lock (bucket); - /* if route currently exists, get the instance which owns it */ he = hash_lookup_fast (m->vhash, bucket, addr, hv); if (he) @@ -966,7 +964,6 @@ multi_learn_addr (struct multi_context *m, gc_free (&gc); } - hash_bucket_unlock (bucket); return owner; } @@ -1130,7 +1127,7 @@ multi_delete_dup (struct multi_context *m, struct multi_instance *new_mi) struct hash_element *he; int count = 0; - hash_iterator_init (m->iter, &hi, true); + hash_iterator_init (m->iter, &hi); while ((he = hash_iterator_next (&hi))) { struct multi_instance *mi = (struct multi_instance *) he->value; @@ -1768,7 +1765,7 @@ multi_bcast (struct multi_context *m, printf ("BCAST len=%d\n", BLEN (buf)); #endif mb = mbuf_alloc_buf (buf); - hash_iterator_init (m->iter, &hi, true); + hash_iterator_init (m->iter, &hi); while ((he = hash_iterator_next (&hi))) { @@ -2462,7 +2459,7 @@ management_callback_kill_by_cn (void *arg, const char *del_cn) struct hash_element *he; int count = 0; - hash_iterator_init (m->iter, &hi, true); + hash_iterator_init (m->iter, &hi); while ((he = hash_iterator_next (&hi))) { struct multi_instance *mi = (struct multi_instance *) he->value; @@ -2496,7 +2493,7 @@ management_callback_kill_by_addr (void *arg, const in_addr_t addr, const int por saddr.sa.sin_port = htons (port); if (mroute_extract_openvpn_sockaddr (&maddr, &saddr, true)) { - hash_iterator_init (m->iter, &hi, true); + hash_iterator_init (m->iter, &hi); while ((he = hash_iterator_next (&hi))) { struct multi_instance *mi = (struct multi_instance *) he->value; diff --git a/pf.c b/pf.c index 027eb695..ee38ca6b 100644 --- a/pf.c +++ b/pf.c @@ -638,7 +638,7 @@ pf_cn_set_print (const struct pf_cn_set *s, const int lev) if (s->hash_table) { - hash_iterator_init (s->hash_table, &hi, false); + hash_iterator_init (s->hash_table, &hi); while ((he = hash_iterator_next (&hi))) { struct pf_cn *e = (struct pf_cn *)he->value; From 2ff54d66a22402caf08709ec22730e20c193ecbd Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Mon, 15 Nov 2010 21:44:59 +0100 Subject: [PATCH 4/9] Use stricter snprintf() formatting in socks_username_password_auth() (v3) commit fc1fa9ffc7e3356458ec3 added a new function which needs to have a stricter string formatting. This was detected due to a compiler warning. This patch makes sure that the length of username and password is not longer than 255 bytes. It also adds extra checks to avoid NULL pointer issues with strlen() on these two parameters. Signed-off-by: David Sommerseth Acked-by: Gert Doering --- socks.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/socks.c b/socks.c index 58b3648b..c7c0473c 100644 --- a/socks.c +++ b/socks.c @@ -112,10 +112,17 @@ socks_username_password_auth (struct socks_proxy_info *p, ssize_t size; creds.defined = 0; - get_user_pass (&creds, p->authfile, UP_TYPE_SOCKS, GET_USER_PASS_MANAGEMENT); - snprintf (to_send, sizeof (to_send), "\x01%c%s%c%s", strlen(creds.username), - creds.username, strlen(creds.password), creds.password); + + if( !creds.username || (strlen(creds.username) > 255) + || !creds.password || (strlen(creds.password) > 255) ) { + msg (M_NONFATAL, + "SOCKS username and/or password exceeds 255 characters. " + "Authentication not possible."); + return false; + } + snprintf (to_send, sizeof (to_send), "\x01%c%s%c%s", (int) strlen(creds.username), + creds.username, (int) strlen(creds.password), creds.password); size = send (sd, to_send, strlen(to_send), MSG_NOSIGNAL); if (size != strlen (to_send)) From eabb8eed0bc3b2b16722eeb38d8000eda35668a7 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Mon, 15 Nov 2010 08:48:57 +0100 Subject: [PATCH 5/9] Fix compiler warnings about not used dummy() functions It has been reported that the Microsoft Visual C compiler complains if a .c file do not contain any compilable code, which can happen if the code has been #ifdef'ed out. To avoid this, these #ifdef sections have a #else section which adds a static dummy() function which does nothing. On the other hand, the GNU C compiler complains about unused functions when it discovers this situation. This patch tries to only add these dummy() functions if the Microsoft Visual C compiler is detected, via the _MSC_VER macro. Signed-off-by: David Sommerseth Acked-by: Peter Stuge --- cryptoapi.c | 2 ++ ieproxy.c | 3 ++- perf.c | 2 ++ pkcs11.c | 2 ++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/cryptoapi.c b/cryptoapi.c index 8fb53871..3365cd7a 100644 --- a/cryptoapi.c +++ b/cryptoapi.c @@ -470,5 +470,7 @@ int SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop) } #else +#ifdef _MSC_VER /* Dummy function needed to avoid empty file compiler warning in Microsoft VC */ static void dummy (void) {} +#endif #endif /* WIN32 */ diff --git a/ieproxy.c b/ieproxy.c index 89977a82..30998704 100644 --- a/ieproxy.c +++ b/ieproxy.c @@ -139,7 +139,8 @@ LPCTSTR getIeHttpProxy() return(NULL); } } - #else +#ifdef _MSC_VER /* Dummy function needed to avoid empty file compiler warning in Microsoft VC */ static void dummy (void) {} +#endif #endif /* WIN32 */ diff --git a/perf.c b/perf.c index a149c07d..67ea9580 100644 --- a/perf.c +++ b/perf.c @@ -287,5 +287,7 @@ perf_print_state (int lev) } #else +#ifdef _MSC_VER /* Dummy function needed to avoid empty file compiler warning in Microsoft VC */ static void dummy(void) {} #endif +#endif diff --git a/pkcs11.c b/pkcs11.c index e06a2ed7..d90ac968 100644 --- a/pkcs11.c +++ b/pkcs11.c @@ -982,5 +982,7 @@ cleanup: } #else +#ifdef _MSC_VER /* Dummy function needed to avoid empty file compiler warning in Microsoft VC */ static void dummy (void) {} +#endif #endif /* ENABLE_PKCS11 */ From 33ee747fff4acb4ea4c143089aa2c596a1e4d0bd Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Mon, 15 Nov 2010 08:53:40 +0100 Subject: [PATCH 6/9] Fixed potential misinterpretation of boolean logic The GNU C compiler warned about a potential issue with an if() expression missing an extra set of parentheses. Signed-off-by: David Sommerseth Acked-by: Peter Stuge --- ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssl.c b/ssl.c index c05d34fc..8644ae44 100644 --- a/ssl.c +++ b/ssl.c @@ -940,7 +940,7 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx) if (opt->verify_export_cert) { gc = gc_new(); - if (tmp_file=get_peer_cert(ctx, opt->verify_export_cert,&gc)) + if ((tmp_file=get_peer_cert(ctx, opt->verify_export_cert,&gc))) { setenv_str(opt->es, "peer_cert", tmp_file); } From d29e6de16ad1cbbc5740e732268da9347b370a1d Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Mon, 15 Nov 2010 08:56:18 +0100 Subject: [PATCH 7/9] Only add some functions when really needed The GNU C compiler gave warnings about some functions not being used. These functions where only used if certian #ifdef sections was enabled. This patch encapsulates these function declarations with matching #ifdef's to make it more clear when these functions are needed. Signed-off-by: David Sommerseth Acked-by: Peter Stuge --- options.c | 2 ++ reliable.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/options.c b/options.c index f0b93100..d0cec7dc 100644 --- a/options.c +++ b/options.c @@ -2785,6 +2785,7 @@ positive_atoi (const char *str) return i < 0 ? 0 : i; } +#ifdef WIN32 /* This function is only used when compiling on Windows */ static unsigned int atou (const char *str) { @@ -2792,6 +2793,7 @@ atou (const char *str) sscanf (str, "%u", &val); return val; } +#endif static inline bool space (unsigned char c) diff --git a/reliable.c b/reliable.c index a41f2bf1..e68c2363 100644 --- a/reliable.c +++ b/reliable.c @@ -516,6 +516,7 @@ reliable_can_send (const struct reliable *rel) return n_current > 0 && !rel->hold; } +#ifdef EXPONENTIAL_BACKOFF /* return a unique point-in-time to trigger retry */ static time_t reliable_unique_retry (struct reliable *rel, time_t retry) @@ -535,6 +536,7 @@ reliable_unique_retry (struct reliable *rel, time_t retry) } return retry; } +#endif /* return next buffer to send to remote */ struct buffer * From 7581c8fd6f0b0f0855b8dae8dba697fecb5ac6a4 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Mon, 15 Nov 2010 08:58:36 +0100 Subject: [PATCH 8/9] Removed functions not being used anywhere The GNU C compiler gave warnings about these functions in the patch not being used anywhere. Doing a git grep on the code turned out there were no callers to these functions. Taking these functions out, as there is not good reason why to carry dead code. Signed-off-by: David Sommerseth Acked-by: Peter Stuge --- ps.c | 12 ------------ sig.c | 9 --------- 2 files changed, 21 deletions(-) diff --git a/ps.c b/ps.c index 003fb45e..0aaef628 100644 --- a/ps.c +++ b/ps.c @@ -234,18 +234,6 @@ port_share_sendmsg (const socket_descriptor_t sd, } } -static int -pc_list_len (struct proxy_connection *pc) -{ - int count = 0; - while (pc) - { - ++count; - pc = pc->next; - } - return count; -} - static void proxy_entry_close_sd (struct proxy_connection *pc, struct event_set *es) { diff --git a/sig.c b/sig.c index 4dd6b092..438f4e66 100644 --- a/sig.c +++ b/sig.c @@ -185,15 +185,6 @@ signal_handler (const int signum) signal (signum, signal_handler); } -/* temporary signal handler, before we are fully initialized */ -static void -signal_handler_exit (const int signum) -{ - msg (M_FATAL, - "Signal %d (%s) received during initialization, exiting", - signum, signal_description (signum, NULL)); -} - #endif /* set handlers for unix signals */ From 7c18c6353904f8c6e7f4eab3d13c985761ab80e5 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Mon, 15 Nov 2010 09:00:12 +0100 Subject: [PATCH 9/9] Merged add_bypass_address() and add_host_route_if_nonlocal() The add_host_route_if_nonlocal() function is too simple to really benefit from calling add_bypass_address() when this function is the only caller to this function. Signed-off-by: David Sommerseth Acked-by: Peter Stuge --- route.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/route.c b/route.c index 612bcbef..f78893ef 100644 --- a/route.c +++ b/route.c @@ -59,26 +59,6 @@ print_bypass_addresses (const struct route_bypass *rb) #endif -static bool -add_bypass_address (struct route_bypass *rb, const in_addr_t a) -{ - int i; - for (i = 0; i < rb->n_bypass; ++i) - { - if (a == rb->bypass[i]) /* avoid duplicates */ - return true; - } - if (rb->n_bypass < N_ROUTE_BYPASS) - { - rb->bypass[rb->n_bypass++] = a; - return true; - } - else - { - return false; - } -} - struct route_option_list * new_route_option_list (const int max_routes, struct gc_arena *a) { @@ -2124,8 +2104,18 @@ netmask_to_netbits (const in_addr_t network, const in_addr_t netmask, int *netbi static void add_host_route_if_nonlocal (struct route_bypass *rb, const in_addr_t addr) { - if (test_local_addr(addr) == TLA_NONLOCAL && addr != 0 && addr != ~0) - add_bypass_address (rb, addr); + if (test_local_addr(addr) == TLA_NONLOCAL && addr != 0 && addr != ~0) { + int i; + for (i = 0; i < rb->n_bypass; ++i) + { + if (addr == rb->bypass[i]) /* avoid duplicates */ + return; + } + if (rb->n_bypass < N_ROUTE_BYPASS) + { + rb->bypass[rb->n_bypass++] = addr; + } + } } static void