mem functions cleanup

Only two macros CRYPTO_MDEBUG and CRYPTO_MDEBUG_ABORT to control this.
If CRYPTO_MDEBUG is not set, #ifdef out the whole debug machinery.
        (Thanks to Jakob Bohm for the suggestion!)
Make the "change wrapper functions" be the only paradigm.
Wrote documentation!
Format the 'set func' functions so their paramlists are legible.
Format some multi-line comments.
Remove ability to get/set the "memory debug" functions at runtme.
Remove MemCheck_* and CRYPTO_malloc_debug_init macros.
Add CRYPTO_mem_debug(int flag) function.
Add test/memleaktest.
Rename CRYPTO_malloc_init to OPENSSL_malloc_init; remove needless calls.

Reviewed-by: Richard Levitte <levitte@openssl.org>

1 files changed, 81 insertions, 156 deletions

diff --git a/crypto/mem_dbg.c b/crypto/mem_dbg.c
index 924652a6cb..2132b7da5c 100644
--- a/crypto/mem_dbg.c
+++ b/crypto/mem_dbg.c
@@ -121,7 +121,6 @@
 # include <execinfo.h>
 #endif
 
-static int mh_mode = CRYPTO_MEM_CHECK_OFF;
 /*
  * The state changes to CRYPTO_MEM_CHECK_ON | CRYPTO_MEM_CHECK_ENABLE when
  * the application asks for it (usually after library initialisation for
@@ -133,14 +132,15 @@ static int mh_mode = CRYPTO_MEM_CHECK_OFF;
  * checking temporarily. State CRYPTO_MEM_CHECK_ENABLE without ..._ON makes
  * no sense whatsoever.
  */
+static int mh_mode = CRYPTO_MEM_CHECK_OFF;
 
+#ifdef CRYPTO_MDEBUG
 static unsigned long order = 0; /* number of memory requests */
 
 DECLARE_LHASH_OF(MEM);
 static LHASH_OF(MEM) *mh = NULL; /* hash-table of memory requests (address as
                                   * key); access requires MALLOC2 lock */
 
-typedef struct app_mem_info_st
 /*-
  * For application-defined information (static C-string `info')
  * to be displayed in memory leak list.
@@ -148,7 +148,7 @@ typedef struct app_mem_info_st
  *   OPENSSL_mem_debug_push("...")     to push an entry,
  *   OPENSSL_mem_debug_pop()     to pop an entry,
  */
-{
+typedef struct app_mem_info_st {
     CRYPTO_THREADID threadid;
     const char *file;
     int line;
@@ -157,18 +157,16 @@ typedef struct app_mem_info_st
     int references;
 } APP_INFO;
 
-static void app_info_free(APP_INFO *);
-
+/*
+ * hash-table with those app_mem_info_st's that are at the
+ * top of their thread's stack (with `thread' as key); access requires
+ * MALLOC2 lock
+ */
 DECLARE_LHASH_OF(APP_INFO);
-static LHASH_OF(APP_INFO) *amih = NULL; /* hash-table with those
-                                         * app_mem_info_st's that are at the
-                                         * top of their thread's stack (with
-                                         * `thread' as key); access requires
-                                         * MALLOC2 lock */
+static LHASH_OF(APP_INFO) *amih = NULL;
 
-typedef struct mem_st
 /* memory-block description */
-{
+typedef struct mem_st {
     void *addr;
     int num;
     const char *file;
@@ -183,17 +181,8 @@ typedef struct mem_st
 #endif
 } MEM;
 
-static long options =           /* extra information to be recorded */
-#if defined(CRYPTO_MDEBUG_TIME) || defined(CRYPTO_MDEBUG_ALL)
-    V_CRYPTO_MDEBUG_TIME |
-#endif
-#if defined(CRYPTO_MDEBUG_THREAD) || defined(CRYPTO_MDEBUG_ALL)
-    V_CRYPTO_MDEBUG_THREAD |
-#endif
-    0;
-
-static unsigned int num_disable = 0; /* num_disable > 0 iff mh_mode ==
-                                      * CRYPTO_MEM_CHECK_ON (w/o ..._ENABLE) */
+/* num_disable > 0 iff mh_mode == CRYPTO_MEM_CHECK_ON (w/o ..._ENABLE) */
+static unsigned int num_disable = 0;
 
 /*
  * Valid iff num_disable > 0.  CRYPTO_LOCK_MALLOC2 is locked exactly in this
@@ -210,29 +199,32 @@ static void app_info_free(APP_INFO *inf)
         OPENSSL_free(inf);
     }
 }
+#endif
 
 int CRYPTO_mem_ctrl(int mode)
 {
+#ifndef CRYPTO_MDEBUG
+    return mode - mode;
+#else
     int ret = mh_mode;
 
     CRYPTO_w_lock(CRYPTO_LOCK_MALLOC);
     switch (mode) {
-        /*
-         * for applications (not to be called while multiple threads use the
-         * library):
-         */
-    case CRYPTO_MEM_CHECK_ON:  /* aka MemCheck_start() */
+    default:
+        break;
+
+    case CRYPTO_MEM_CHECK_ON:
         mh_mode = CRYPTO_MEM_CHECK_ON | CRYPTO_MEM_CHECK_ENABLE;
         num_disable = 0;
         break;
-    case CRYPTO_MEM_CHECK_OFF: /* aka MemCheck_stop() */
+
+    case CRYPTO_MEM_CHECK_OFF:
         mh_mode = 0;
-        num_disable = 0;        /* should be true *before* MemCheck_stop is
-                                 * used, or there'll be a lot of confusion */
+        num_disable = 0;
         break;
 
-        /* switch off temporarily (for library-internal use): */
-    case CRYPTO_MEM_CHECK_DISABLE: /* aka MemCheck_off() */
+    /* switch off temporarily (for library-internal use): */
+    case CRYPTO_MEM_CHECK_DISABLE:
         if (mh_mode & CRYPTO_MEM_CHECK_ON) {
             CRYPTO_THREADID cur;
             CRYPTO_THREADID_current(&cur);
@@ -262,7 +254,8 @@ int CRYPTO_mem_ctrl(int mode)
             num_disable++;
         }
         break;
-    case CRYPTO_MEM_CHECK_ENABLE: /* aka MemCheck_on() */
+
+    case CRYPTO_MEM_CHECK_ENABLE:
         if (mh_mode & CRYPTO_MEM_CHECK_ON) {
             if (num_disable) {  /* always true, or something is going wrong */
                 num_disable--;
@@ -273,15 +266,15 @@ int CRYPTO_mem_ctrl(int mode)
             }
         }
         break;
-
-    default:
-        break;
     }
     CRYPTO_w_unlock(CRYPTO_LOCK_MALLOC);
     return (ret);
+#endif
 }
 
-int CRYPTO_is_mem_check_on(void)
+#ifdef CRYPTO_MDEBUG
+
+static int mem_check_on(void)
 {
     int ret = 0;
 
@@ -298,16 +291,6 @@ int CRYPTO_is_mem_check_on(void)
     return (ret);
 }
 
-void CRYPTO_dbg_set_options(long bits)
-{
-    options = bits;
-}
-
-long CRYPTO_dbg_get_options(void)
-{
-    return options;
-}
-
 static int mem_cmp(const MEM *a, const MEM *b)
 {
 #ifdef _WIN64
@@ -388,17 +371,14 @@ int CRYPTO_mem_debug_push(const char *info, const char *file, int line)
     APP_INFO *ami, *amim;
     int ret = 0;
 
-    if (is_MemCheck_on()) {
-        MemCheck_off();         /* obtain MALLOC2 lock */
+    if (mem_check_on()) {
+        CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
 
-        if ((ami = OPENSSL_malloc(sizeof(*ami))) == NULL) {
-            ret = 0;
+        if ((ami = OPENSSL_malloc(sizeof(*ami))) == NULL)
             goto err;
-        }
         if (amih == NULL) {
             if ((amih = lh_APP_INFO_new()) == NULL) {
                 OPENSSL_free(ami);
-                ret = 0;
                 goto err;
             }
         }
@@ -412,8 +392,9 @@ int CRYPTO_mem_debug_push(const char *info, const char *file, int line)
 
         if ((amim = lh_APP_INFO_insert(amih, ami)) != NULL)
             ami->next = amim;
+        ret = 1;
  err:
-        MemCheck_on();          /* release MALLOC2 lock */
+        CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
     }
 
     return (ret);
@@ -423,20 +404,18 @@ int CRYPTO_mem_debug_pop(void)
 {
     int ret = 0;
 
-    if (is_MemCheck_on()) {     /* _must_ be true, or something went severely
-                                 * wrong */
-        MemCheck_off();         /* obtain MALLOC2 lock */
-
+    if (mem_check_on()) {
+        CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
         ret = (pop_info() != NULL);
-
-        MemCheck_on();          /* release MALLOC2 lock */
+        CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
     }
     return (ret);
 }
 
 static unsigned long break_order_num = 0;
-void CRYPTO_dbg_malloc(void *addr, size_t num, const char *file, int line,
-                       int before_p)
+
+void CRYPTO_mem_debug_malloc(void *addr, size_t num, int before_p,
+                             const char *file, int line)
 {
     MEM *m, *mm;
     APP_INFO tmp, *amim;
@@ -448,12 +427,11 @@ void CRYPTO_dbg_malloc(void *addr, size_t num, const char *file, int line,
         if (addr == NULL)
             break;
 
-        if (is_MemCheck_on()) {
-            MemCheck_off();     /* make sure we hold MALLOC2 lock */
+        if (mem_check_on()) {
+            CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
             if ((m = OPENSSL_malloc(sizeof(*m))) == NULL) {
                 OPENSSL_free(addr);
-                MemCheck_on();  /* release MALLOC2 lock if num_disabled drops
-                                 * to 0 */
+                CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
                 return;
             }
             if (mh == NULL) {
@@ -469,23 +447,17 @@ void CRYPTO_dbg_malloc(void *addr, size_t num, const char *file, int line,
             m->file = file;
             m->line = line;
             m->num = num;
-            if (options & V_CRYPTO_MDEBUG_THREAD)
-                CRYPTO_THREADID_current(&m->threadid);
-            else
-                memset(&m->threadid, 0, sizeof(m->threadid));
+            CRYPTO_THREADID_current(&m->threadid);
 
             if (order == break_order_num) {
                 /* BREAK HERE */
                 m->order = order;
             }
             m->order = order++;
-            if (options & V_CRYPTO_MDEBUG_TIME)
-                m->time = time(NULL);
-            else
-                m->time = 0;
-#if defined(CRYPTO_MDEBUG_BACKTRACE) && defined(__GNUC__)
+# if defined(CRYPTO_MDEBUG_BACKTRACE) && defined(__GNUC__)
             m->array_siz = backtrace(m->array, OSSL_NELEM(m->array));
-#endif
+# endif
+            m->time = time(NULL);
 
             CRYPTO_THREADID_current(&tmp.threadid);
             m->app_info = NULL;
@@ -503,15 +475,14 @@ void CRYPTO_dbg_malloc(void *addr, size_t num, const char *file, int line,
                 OPENSSL_free(mm);
             }
  err:
-            MemCheck_on();      /* release MALLOC2 lock if num_disabled drops
-                                 * to 0 */
+            CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
         }
         break;
     }
     return;
 }
 
-void CRYPTO_dbg_free(void *addr, int before_p)
+void CRYPTO_mem_debug_free(void *addr, int before_p)
 {
     MEM m, *mp;
 
@@ -520,8 +491,8 @@ void CRYPTO_dbg_free(void *addr, int before_p)
         if (addr == NULL)
             break;
 
-        if (is_MemCheck_on() && (mh != NULL)) {
-            MemCheck_off();     /* make sure we hold MALLOC2 lock */
+        if (mem_check_on() && (mh != NULL)) {
+            CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
 
             m.addr = addr;
             mp = lh_MEM_delete(mh, &m);
@@ -530,8 +501,7 @@ void CRYPTO_dbg_free(void *addr, int before_p)
                 OPENSSL_free(mp);
             }
 
-            MemCheck_on();      /* release MALLOC2 lock if num_disabled drops
-                                 * to 0 */
+            CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
         }
         break;
     case 1:
@@ -539,8 +509,8 @@ void CRYPTO_dbg_free(void *addr, int before_p)
     }
 }
 
-void CRYPTO_dbg_realloc(void *addr1, void *addr2, size_t num,
-                        const char *file, int line, int before_p)
+void CRYPTO_mem_debug_realloc(void *addr1, void *addr2, size_t num,
+                              int before_p, const char *file, int line)
 {
     MEM m, *mp;
 
@@ -552,12 +522,12 @@ void CRYPTO_dbg_realloc(void *addr1, void *addr2, size_t num,
             break;
 
         if (addr1 == NULL) {
-            CRYPTO_dbg_malloc(addr2, num, file, line, 128 | before_p);
+            CRYPTO_mem_debug_malloc(addr2, num, 128 | before_p, file, line);
             break;
         }
 
-        if (is_MemCheck_on()) {
-            MemCheck_off();     /* make sure we hold MALLOC2 lock */
+        if (mem_check_on()) {
+            CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
 
             m.addr = addr1;
             mp = lh_MEM_delete(mh, &m);
@@ -570,8 +540,7 @@ void CRYPTO_dbg_realloc(void *addr1, void *addr2, size_t num,
                 (void)lh_MEM_insert(mh, mp);
             }
 
-            MemCheck_on();      /* release MALLOC2 lock if num_disabled drops
-                                 * to 0 */
+            CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
         }
         break;
     }
@@ -602,23 +571,18 @@ static void print_leak_doall_arg(const MEM *m, MEM_LEAK *l)
         return;
     }
 
-    if (options & V_CRYPTO_MDEBUG_TIME) {
-        lcl = localtime(&m->time);
-
-        BIO_snprintf(bufp, BUF_REMAIN, "[%02d:%02d:%02d] ",
-                     lcl->tm_hour, lcl->tm_min, lcl->tm_sec);
-        bufp += strlen(bufp);
-    }
+    lcl = localtime(&m->time);
+    BIO_snprintf(bufp, BUF_REMAIN, "[%02d:%02d:%02d] ",
+                 lcl->tm_hour, lcl->tm_min, lcl->tm_sec);
+    bufp += strlen(bufp);
 
     BIO_snprintf(bufp, BUF_REMAIN, "%5lu file=%s, line=%d, ",
                  m->order, m->file, m->line);
     bufp += strlen(bufp);
 
-    if (options & V_CRYPTO_MDEBUG_THREAD) {
-        BIO_snprintf(bufp, BUF_REMAIN, "thread=%lu, ",
-                     CRYPTO_THREADID_hash(&m->threadid));
-        bufp += strlen(bufp);
-    }
+    BIO_snprintf(bufp, BUF_REMAIN, "thread=%lu, ",
+                 CRYPTO_THREADID_hash(&m->threadid));
+    bufp += strlen(bufp);
 
     BIO_snprintf(bufp, BUF_REMAIN, "number=%d, address=%p\n",
                  m->num, m->addr);
@@ -631,6 +595,7 @@ static void print_leak_doall_arg(const MEM *m, MEM_LEAK *l)
 
     amip = m->app_info;
     ami_cnt = 0;
+
     if (amip) {
         CRYPTO_THREADID_cpy(&ti, &amip->threadid);
 
@@ -666,9 +631,9 @@ static void print_leak_doall_arg(const MEM *m, MEM_LEAK *l)
     {
         size_t i;
         char **strings = backtrace_symbols(m->array, m->array_siz);
+
         for (i = 0; i < m->array_siz; i++)
             fprintf(stderr, "##> %s\n", strings[i]);
-
         free(strings);
     }
 #endif
@@ -683,7 +648,7 @@ void CRYPTO_mem_leaks(BIO *b)
     if (mh == NULL && amih == NULL)
         return;
 
-    MemCheck_off();             /* obtain MALLOC2 lock */
+    CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
 
     ml.bio = b;
     ml.bytes = 0;
@@ -698,29 +663,23 @@ void CRYPTO_mem_leaks(BIO *b)
     }
     if (ml.chunks != 0) {
         BIO_printf(b, "%ld bytes leaked in %d chunks\n", ml.bytes, ml.chunks);
-#ifdef CRYPTO_MDEBUG_ABORT
+# ifdef CRYPTO_MDEBUG_ABORT
         abort();
-#endif
+# endif
     } else {
         /*
          * Make sure that, if we found no leaks, memory-leak debugging itself
          * does not introduce memory leaks (which might irritate external
          * debugging tools). (When someone enables leak checking, but does not
-         * call this function, we declare it to be their fault.) XXX This
-         * should be in CRYPTO_mem_leaks_cb, and CRYPTO_mem_leaks should be
-         * implemented by using CRYPTO_mem_leaks_cb. (Also there should be a
-         * variant of lh_doall_arg that takes a function pointer instead of a
-         * void *; this would obviate the ugly and illegal void_fn_to_char
-         * kludge in CRYPTO_mem_leaks_cb. Otherwise the code police will come
-         * and get us.)
+         * call this function, we declare it to be their fault.)
          */
         int old_mh_mode;
 
         CRYPTO_w_lock(CRYPTO_LOCK_MALLOC);
 
         /*
-         * avoid deadlock when lh_free() uses CRYPTO_dbg_free(), which uses
-         * CRYPTO_is_mem_check_on
+         * avoid deadlock when lh_free() uses CRYPTO_mem_debug_free(), which uses
+         * mem_check_on
          */
         old_mh_mode = mh_mode;
         mh_mode = CRYPTO_MEM_CHECK_OFF;
@@ -737,10 +696,10 @@ void CRYPTO_mem_leaks(BIO *b)
         mh_mode = old_mh_mode;
         CRYPTO_w_unlock(CRYPTO_LOCK_MALLOC);
     }
-    MemCheck_on();              /* release MALLOC2 lock */
+    CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
 }
 
-#ifndef OPENSSL_NO_STDIO
+# ifndef OPENSSL_NO_STDIO
 void CRYPTO_mem_leaks_fp(FILE *fp)
 {
     BIO *b;
@@ -752,49 +711,15 @@ void CRYPTO_mem_leaks_fp(FILE *fp)
      * we're creating them at a time when we're trying to check we've not
      * left anything un-free()'d!!
      */
-    MemCheck_off();
+    CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
     b = BIO_new(BIO_s_file());
-    MemCheck_on();
+    CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
     if (b == NULL)
         return;
     BIO_set_fp(b, fp, BIO_NOCLOSE);
     CRYPTO_mem_leaks(b);
     BIO_free(b);
 }
-#endif
-
-/*
- * FIXME: We really don't allow much to the callback.  For example, it has no
- * chance of reaching the info stack for the item it processes.  Should it
- * really be this way? -- Richard Levitte
- */
-/*
- * NB: The prototypes have been typedef'd to CRYPTO_MEM_LEAK_CB inside
- * crypto.h If this code is restructured, remove the callback type if it is
- * no longer needed. -- Geoff Thorpe
- */
-
-/*
- * Can't pass CRYPTO_MEM_LEAK_CB directly to lh_MEM_doall_arg because it is a
- * function pointer and conversion to void * is prohibited. Instead pass its
- * address
- */
-
-typedef CRYPTO_MEM_LEAK_CB *PCRYPTO_MEM_LEAK_CB;
-
-static void cb_leak_doall_arg(const MEM *m, PCRYPTO_MEM_LEAK_CB *cb)
-{
-    (*cb) (m->order, m->file, m->line, m->num, m->addr);
-}
+# endif
 
-static IMPLEMENT_LHASH_DOALL_ARG_FN(cb_leak, const MEM, PCRYPTO_MEM_LEAK_CB)
-
-void CRYPTO_mem_leaks_cb(CRYPTO_MEM_LEAK_CB *cb)
-{
-    if (mh == NULL)
-        return;
-    CRYPTO_w_lock(CRYPTO_LOCK_MALLOC2);
-    lh_MEM_doall_arg(mh, LHASH_DOALL_ARG_FN(cb_leak), PCRYPTO_MEM_LEAK_CB,
-                     &cb);
-    CRYPTO_w_unlock(CRYPTO_LOCK_MALLOC2);
-}
+#endif