Bojan Smojver wrote:

Given we use CTR, maybe I can just commit something like this to trunk
and others can then commit improvements over the top, if desired?

  I'm an httpd guy but not a APR one, so it's really up to you!

  Here's a slight variant which, I believe, bypasses
apr_generate_random_bytes() unless you've got /dev/urandom or /dev/arandom.

  I think that might be useful because, looking at apr_generate_random_bytes()
in the cases where you don't have DEV_RANDOM set but do have things like
HAVE_EGD or HAVE_TRUERAND, it seems like most of these other options,
like /dev/random, are set to block, e.g., comments like:

   req[0] = 2; /* We'll block for now. */

   /* this will increase the startup time of the server, unfortunately...
    * (generating 20 bytes takes about 8 seconds)

  Anyway, it seems like the best might just be to skip the thing
unless we're guaranteed a non-blocking action.

  I also tried to fix a compiler warning on the cast from the pointer ht
into an integer ... others should test on differently-sized systems,
but it was throwing "cast from pointer to integer of different size"
for an older 32-bit system with the cast to apr_time_t.  Please test
and improve!  Thanks,

Chris.

===============================================
--- tables/apr_hash.c.orig      2012-01-10 11:47:17.000000000 -0800
+++ tables/apr_hash.c   2012-01-12 19:55:43.000000000 -0800
@@ -18,6 +18,10 @@

#include "apr_general.h"
#include "apr_pools.h"
+#include "apr_time.h"
+#if APR_HAVE_STDLIB_H
+#include <stdlib.h>     /* for rand, srand */
+#endif

#include "apr_hash.h"

@@ -75,7 +79,7 @@
    apr_pool_t          *pool;
    apr_hash_entry_t   **array;
    apr_hash_index_t     iterator;  /* For apr_hash_first(NULL, ...) */
-    unsigned int         count, max;
+    unsigned int         count, max, seed;
    apr_hashfunc_t       hash_func;
    apr_hash_entry_t    *free;  /* List of recycled entries */
};
@@ -95,13 +99,28 @@
APR_DECLARE(apr_hash_t *) apr_hash_make(apr_pool_t *pool)
{
    apr_hash_t *ht;
+    apr_time_t now;
+
    ht = apr_palloc(pool, sizeof(apr_hash_t));
    ht->pool = pool;
    ht->free = NULL;
    ht->count = 0;
    ht->max = INITIAL_MAX;
+#if APR_HAS_RANDOM
+#ifdef DEV_RANDOM
+    if (strcmp(DEV_RANDOM, "/dev/random") != 0 &&
+        apr_generate_random_bytes((unsigned char *)&ht->seed,
+                                  sizeof(ht->seed)) != APR_SUCCESS)
+#endif
+#endif
+    {
+        now = apr_time_now();
+        srand((unsigned int)((now >> 32) ^ now ^ (apr_uintptr_t)ht));
+        ht->seed = (unsigned int)(rand());
+    }
    ht->array = alloc_array(ht, ht->max);
-    ht->hash_func = apr_hashfunc_default;
+    ht->hash_func = NULL;
+
    return ht;
}

@@ -201,10 +220,10 @@
    ht->max = new_max;
}

-APR_DECLARE_NONSTD(unsigned int) apr_hashfunc_default(const char *char_key,
-                                                      apr_ssize_t *klen)
+static unsigned int apr_hashfunc_default_internal(const char *char_key,
+                                                  apr_ssize_t *klen,
+                                                  unsigned int hash)
{
-    unsigned int hash = 0;
    const unsigned char *key = (const unsigned char *)char_key;
    const unsigned char *p;
    apr_ssize_t i;
@@ -246,7 +265,7 @@
     *
     *                  -- Ralf S. Engelschall <r...@engelschall.com>
     */
- +
    if (*klen == APR_HASH_KEY_STRING) {
        for (p = key; *p; p++) {
            hash = hash * 33 + *p;
@@ -262,6 +281,11 @@
    return hash;
}

+APR_DECLARE_NONSTD(unsigned int) apr_hashfunc_default(const char *char_key,
+                                                      apr_ssize_t *klen)
+{
+    return apr_hashfunc_default_internal(char_key, klen, 0);
+}

/*
 * This is where we keep the details of the hash function and control
@@ -280,7 +304,10 @@
    apr_hash_entry_t **hep, *he;
    unsigned int hash;

-    hash = ht->hash_func(key, &klen);
+    if (ht->hash_func)
+        hash = ht->hash_func(key, &klen);
+    else
+        hash = apr_hashfunc_default_internal(key, &klen, ht->seed);

    /* scan linked list */
    for (hep = &ht->array[hash & ht->max], he = *hep;
@@ -322,6 +349,7 @@
    ht->free = NULL;
    ht->count = orig->count;
    ht->max = orig->max;
+    ht->seed = orig->seed;
    ht->hash_func = orig->hash_func;
    ht->array = (apr_hash_entry_t **)((char *)ht + sizeof(apr_hash_t));

Reply via email to