First step in tidying up the LHASH code. The callback prototypes (and

casts) used in the lhash code are about as horrible and evil as they can
be. For starters, the callback prototypes contain empty parameter lists.
Yuck.

This first change defines clearer prototypes - including "typedef"'d
function pointer types to use as "hash" and "compare" callbacks, as well as
the callbacks passed to the lh_doall and lh_doall_arg iteration functions.
Now at least more explicit (and clear) casting is required in all of the
dependant code - and that should be included in this commit.

The next step will be to hunt down and obliterate some of the function
pointer casting being used when it's not necessary - a particularly evil
variant exists in the implementation of lh_doall.

apps/ca.c

@@ -753,15 +753,17 @@ bad:
 		BIO_printf(bio_err,"generating index\n");
 		}
 	
-	if (!TXT_DB_create_index(db,DB_serial,NULL,index_serial_hash,
-		index_serial_cmp))
+	if (!TXT_DB_create_index(db, DB_serial, NULL,
+			(LHASH_HASH_FN_TYPE)index_serial_hash,
+			(LHASH_COMP_FN_TYPE)index_serial_cmp))
 		{
 		BIO_printf(bio_err,"error creating serial number index:(%ld,%ld,%ld)\n",db->error,db->arg1,db->arg2);
 		goto err;
 		}
 
-	if (!TXT_DB_create_index(db,DB_name,index_name_qual,index_name_hash,
-		index_name_cmp))
+	if (!TXT_DB_create_index(db, DB_name, index_name_qual,
+			(LHASH_HASH_FN_TYPE)index_name_hash,
+			(LHASH_COMP_FN_TYPE)index_name_cmp))
 		{
 		BIO_printf(bio_err,"error creating name index:(%ld,%ld,%ld)\n",
 			db->error,db->arg1,db->arg2);

apps/openssl.c

@@ -351,7 +351,9 @@ static LHASH *prog_init(void)
 	    ;
 	qsort(functions,i,sizeof *functions,SortFnByName);
 
-	if ((ret=lh_new(hash,cmp)) == NULL) return(NULL);
+	if ((ret=lh_new((LHASH_HASH_FN_TYPE)hash,
+			(LHASH_COMP_FN_TYPE)cmp)) == NULL)
+		return(NULL);
 
 	for (f=functions; f->name != NULL; f++)
 		lh_insert(ret,f);

crypto/conf/cnf_save.c

@@ -73,7 +73,7 @@ main()
 		exit(1);
 		}
 
-	lh_doall(conf,print_conf);
+	lh_doall(conf,(LHASH_DOALL_FN_TYPE)print_conf);
 	}
 
 

crypto/conf/conf_api.c

@@ -181,7 +181,8 @@ int _CONF_new_data(CONF *conf)
 		return 0;
 		}
 	if (conf->data == NULL)
-		if ((conf->data = lh_new(hash,cmp_conf)) == NULL)
+		if ((conf->data = lh_new((LHASH_HASH_FN_TYPE)hash,
+					(LHASH_COMP_FN_TYPE)cmp_conf)) == NULL)
 			{
 			return 0;
 			}
@@ -194,12 +195,14 @@ void _CONF_free_data(CONF *conf)
 
 	conf->data->down_load=0; /* evil thing to make sure the 'OPENSSL_free()'
 				  * works as expected */
-	lh_doall_arg(conf->data,(void (*)())value_free_hash,conf->data);
+	lh_doall_arg(conf->data, (LHASH_DOALL_ARG_FN_TYPE)value_free_hash,
+			conf->data);
 
 	/* We now have only 'section' entries in the hash table.
 	 * Due to problems with */
 
-	lh_doall_arg(conf->data,(void (*)())value_free_stack,conf->data);
+	lh_doall_arg(conf->data, (LHASH_DOALL_ARG_FN_TYPE)value_free_stack,
+			conf->data);
 	lh_free(conf->data);
 	}
 

crypto/conf/conf_def.c

@@ -712,7 +712,7 @@ static void dump_value(CONF_VALUE *a, BIO *out)
 
 static int def_dump(CONF *conf, BIO *out)
 	{
-	lh_doall_arg(conf->data, (void (*)())dump_value, out);
+	lh_doall_arg(conf->data, (LHASH_DOALL_ARG_FN_TYPE)dump_value, out);
 	return 1;
 	}
 

crypto/err/err.c

@@ -316,7 +316,8 @@ void ERR_load_strings(int lib, ERR_STRING_DATA *str)
 	if (error_hash == NULL)
 		{
 		CRYPTO_w_lock(CRYPTO_LOCK_ERR_HASH);
-		error_hash=lh_new(err_hash,err_cmp);
+		error_hash=lh_new((LHASH_HASH_FN_TYPE)err_hash,
+				(LHASH_COMP_FN_TYPE)err_cmp);
 		if (error_hash == NULL)
 			{
 			CRYPTO_w_unlock(CRYPTO_LOCK_ERR_HASH);
@@ -706,7 +707,8 @@ ERR_STATE *ERR_get_state(void)
 		/* no entry yet in thread_hash for current thread -
 		 * thus, it may have changed since we last looked at it */
 		if (thread_hash == NULL)
-			thread_hash = lh_new(pid_hash, pid_cmp);
+			thread_hash = lh_new((LHASH_HASH_FN_TYPE)pid_hash,
+					(LHASH_COMP_FN_TYPE)pid_cmp);
 		if (thread_hash == NULL)
 			thread_state_exists = 0; /* allocation error */
 		else

crypto/lhash/lhash.c

@@ -111,7 +111,7 @@ static void expand(LHASH *lh);
 static void contract(LHASH *lh);
 static LHASH_NODE **getrn(LHASH *lh, void *data, unsigned long *rhash);
 
-LHASH *lh_new(unsigned long (*h)(), int (*c)())
+LHASH *lh_new(LHASH_HASH_FN_TYPE h, LHASH_COMP_FN_TYPE c)
 	{
 	LHASH *ret;
 	int i;
@@ -122,8 +122,8 @@ LHASH *lh_new(unsigned long (*h)(), int (*c)())
 		goto err1;
 	for (i=0; i<MIN_NODES; i++)
 		ret->b[i]=NULL;
-	ret->comp=((c == NULL)?(int (*)())strcmp:c);
-	ret->hash=((h == NULL)?(unsigned long (*)())lh_strhash:h);
+	ret->comp=((c == NULL)?(LHASH_COMP_FN_TYPE)strcmp:c);
+	ret->hash=((h == NULL)?(LHASH_HASH_FN_TYPE)lh_strhash:h);
 	ret->num_nodes=MIN_NODES/2;
 	ret->num_alloc_nodes=MIN_NODES;
 	ret->p=0;
@@ -267,12 +267,19 @@ void *lh_retrieve(LHASH *lh, void *data)
 	return(ret);
 	}
 
-void lh_doall(LHASH *lh, void (*func)())
+void lh_doall(LHASH *lh, LHASH_DOALL_FN_TYPE func)
 	{
-	lh_doall_arg(lh,func,NULL);
+	/* Yikes that's bad - we're accepting a function that accepts 2
+	 * parameters (albeit we have to waive type-safety here) and then
+	 * forcibly calling that callback with *3* parameters leaving the 3rd
+	 * NULL. Obviously this "works" otherwise it wouldn't have survived so
+	 * long, but is it "good"??
+	 * FIXME: Use an internal function from this and the "_arg" version that
+	 * doesn't assume the ability to mutate function prototypes so badly. */
+	lh_doall_arg(lh, (LHASH_DOALL_ARG_FN_TYPE)func, NULL);
 	}
 
-void lh_doall_arg(LHASH *lh, void (*func)(), void *arg)
+void lh_doall_arg(LHASH *lh, LHASH_DOALL_ARG_FN_TYPE func, void *arg)
 	{
 	int i;
 	LHASH_NODE *a,*n;
@@ -312,7 +319,7 @@ static void expand(LHASH *lh)
 #ifndef NO_HASH_COMP
 		hash=np->hash;
 #else
-		hash=(*(lh->hash))(np->data);
+		hash=lh->hash(np->data);
 		lh->num_hash_calls++;
 #endif
 		if ((hash%nni) != p)
@@ -415,7 +422,7 @@ static LHASH_NODE **getrn(LHASH *lh, void *data, unsigned long *rhash)
 			}
 #endif
 		lh->num_comp_calls++;
-		if ((*cf)(n1->data,data) == 0)
+		if(cf(n1->data,data) == 0)
 			break;
 		ret= &(n1->next);
 		}

crypto/lhash/lhash.h

@@ -84,11 +84,16 @@ typedef struct lhash_node_st
 #endif
 	} LHASH_NODE;
 
+typedef int (*LHASH_COMP_FN_TYPE)(void *, void *);
+typedef unsigned long (*LHASH_HASH_FN_TYPE)(void *);
+typedef void (*LHASH_DOALL_FN_TYPE)(void *);
+typedef void (*LHASH_DOALL_ARG_FN_TYPE)(void *, void *);
+
 typedef struct lhash_st
 	{
 	LHASH_NODE **b;
-	int (*comp)();
-	unsigned long (*hash)();
+	LHASH_COMP_FN_TYPE comp;
+	LHASH_HASH_FN_TYPE hash;
 	unsigned int num_nodes;
 	unsigned int num_alloc_nodes;
 	unsigned int p;
@@ -120,13 +125,13 @@ typedef struct lhash_st
  * in lh_insert(). */
 #define lh_error(lh)	((lh)->error)
 
-LHASH *lh_new(unsigned long (*h)(/* void *a */), int (*c)(/* void *a,void *b */));
+LHASH *lh_new(LHASH_HASH_FN_TYPE h, LHASH_COMP_FN_TYPE c);
 void lh_free(LHASH *lh);
 void *lh_insert(LHASH *lh, void *data);
 void *lh_delete(LHASH *lh, void *data);
 void *lh_retrieve(LHASH *lh, void *data);
-    void lh_doall(LHASH *lh, void (*func)(/*void *b*/));
-void lh_doall_arg(LHASH *lh, void (*func)(/*void *a,void *b*/),void *arg);
+void lh_doall(LHASH *lh, LHASH_DOALL_FN_TYPE func);
+void lh_doall_arg(LHASH *lh, LHASH_DOALL_ARG_FN_TYPE func, void *arg);
 unsigned long lh_strhash(const char *c);
 unsigned long lh_num_items(const LHASH *lh);
 

crypto/mem_dbg.c

@@ -302,7 +302,8 @@ int CRYPTO_push_info_(const char *info, const char *file, int line)
 			}
 		if (amih == NULL)
 			{
-			if ((amih=lh_new(app_info_hash,app_info_cmp)) == NULL)
+			if ((amih=lh_new((LHASH_HASH_FN_TYPE)app_info_hash,
+					(LHASH_COMP_FN_TYPE)app_info_cmp)) == NULL)
 				{
 				OPENSSL_free(ami);
 				ret=0;
@@ -394,7 +395,8 @@ void CRYPTO_dbg_malloc(void *addr, int num, const char *file, int line,
 				}
 			if (mh == NULL)
 				{
-				if ((mh=lh_new(mem_hash,mem_cmp)) == NULL)
+				if ((mh=lh_new((LHASH_HASH_FN_TYPE)mem_hash,
+					(LHASH_COMP_FN_TYPE)mem_cmp)) == NULL)
 					{
 					OPENSSL_free(addr);
 					OPENSSL_free(m);
@@ -647,7 +649,8 @@ void CRYPTO_mem_leaks(BIO *b)
 	ml.chunks=0;
 	MemCheck_off(); /* obtains CRYPTO_LOCK_MALLOC2 */
 	if (mh != NULL)
-		lh_doall_arg(mh,(void (*)())print_leak,(char *)&ml);
+		lh_doall_arg(mh, (LHASH_DOALL_ARG_FN_TYPE)print_leak,
+				(char *)&ml);
 	if (ml.chunks != 0)
 		{
 		sprintf(buf,"%ld bytes leaked in %d chunks\n",
@@ -725,6 +728,6 @@ void CRYPTO_mem_leaks_cb(void (*cb)(unsigned long, const char *, int, int, void
 	{
 	if (mh == NULL) return;
 	CRYPTO_w_lock(CRYPTO_LOCK_MALLOC2);
-	lh_doall_arg(mh,(void (*)())cb_leak,(void *)&cb);
+	lh_doall_arg(mh, (LHASH_DOALL_ARG_FN_TYPE)cb_leak,(void *)&cb);
 	CRYPTO_w_unlock(CRYPTO_LOCK_MALLOC2);
 	}

crypto/objects/o_names.c

@@ -31,7 +31,8 @@ int OBJ_NAME_init(void)
 	{
 	if (names_lh != NULL) return(1);
 	MemCheck_off();
-	names_lh=lh_new(obj_name_hash,obj_name_cmp);
+	names_lh=lh_new((LHASH_HASH_FN_TYPE)obj_name_hash,
+			(LHASH_COMP_FN_TYPE)obj_name_cmp);
 	MemCheck_on();
 	return(names_lh != NULL);
 	}
@@ -245,7 +246,7 @@ void OBJ_NAME_do_all(int type,void (*fn)(const OBJ_NAME *,void *arg),void *arg)
 	d.fn=fn;
 	d.arg=arg;
 
-	lh_doall_arg(names_lh,do_all_fn,&d);
+	lh_doall_arg(names_lh,(LHASH_DOALL_ARG_FN_TYPE)do_all_fn,&d);
 	}
 
 struct doall_sorted
@@ -320,7 +321,7 @@ void OBJ_NAME_cleanup(int type)
 	down_load=names_lh->down_load;
 	names_lh->down_load=0;
 
-	lh_doall(names_lh,names_lh_free);
+	lh_doall(names_lh,(LHASH_DOALL_FN_TYPE)names_lh_free);
 	if (type < 0)
 		{
 		lh_free(names_lh);

crypto/objects/obj_dat.c

@@ -177,7 +177,7 @@ static int add_cmp(ADDED_OBJ *ca, ADDED_OBJ *cb)
 static int init_added(void)
 	{
 	if (added != NULL) return(1);
-	added=lh_new(add_hash,add_cmp);
+	added=lh_new((LHASH_HASH_FN_TYPE)add_hash,(LHASH_COMP_FN_TYPE)add_cmp);
 	return(added != NULL);
 	}
 
@@ -203,9 +203,9 @@ void OBJ_cleanup(void)
 	{
 	if (added == NULL) return;
 	added->down_load=0;
-	lh_doall(added,cleanup1); /* zero counters */
-	lh_doall(added,cleanup2); /* set counters */
-	lh_doall(added,cleanup3); /* free objects */
+	lh_doall(added,(LHASH_DOALL_FN_TYPE)cleanup1); /* zero counters */
+	lh_doall(added,(LHASH_DOALL_FN_TYPE)cleanup2); /* set counters */
+	lh_doall(added,(LHASH_DOALL_FN_TYPE)cleanup3); /* free objects */
 	lh_free(added);
 	added=NULL;
 	}

crypto/txt_db/txt_db.c

@@ -211,7 +211,7 @@ char **TXT_DB_get_by_index(TXT_DB *db, int idx, char **value)
 	}
 
 int TXT_DB_create_index(TXT_DB *db, int field, int (*qual)(),
-	     unsigned long (*hash)(), int (*cmp)())
+		LHASH_HASH_FN_TYPE hash, LHASH_COMP_FN_TYPE cmp)
 	{
 	LHASH *idx;
 	char *r;

crypto/txt_db/txt_db.h

@@ -96,7 +96,7 @@ TXT_DB *TXT_DB_read(char *in, int num);
 long TXT_DB_write(char *out, TXT_DB *db);
 #endif
 int TXT_DB_create_index(TXT_DB *db,int field,int (*qual)(),
-	 unsigned long (*hash)(),int (*cmp)());
+		LHASH_HASH_FN_TYPE hash, LHASH_COMP_FN_TYPE cmp);
 void TXT_DB_free(TXT_DB *db);
 char **TXT_DB_get_by_index(TXT_DB *db, int idx, char **value);
 int TXT_DB_insert(TXT_DB *db,char **value);

ssl/ssl_lib.c

@@ -1164,7 +1164,8 @@ SSL_CTX *SSL_CTX_new(SSL_METHOD *meth)
 	ret->default_passwd_callback_userdata=NULL;
 	ret->client_cert_cb=NULL;
 
-	ret->sessions=lh_new(SSL_SESSION_hash,SSL_SESSION_cmp);
+	ret->sessions=lh_new((LHASH_HASH_FN_TYPE)SSL_SESSION_hash,
+			(LHASH_COMP_FN_TYPE)SSL_SESSION_cmp);
 	if (ret->sessions == NULL) goto err;
 	ret->cert_store=X509_STORE_new();
 	if (ret->cert_store == NULL) goto err;

ssl/ssl_sess.c

@@ -606,7 +606,7 @@ void SSL_CTX_flush_sessions(SSL_CTX *s, long t)
 	CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX);
 	i=tp.cache->down_load;
 	tp.cache->down_load=0;
-	lh_doall_arg(tp.cache,(void (*)())timeout,&tp);
+	lh_doall_arg(tp.cache, (LHASH_DOALL_ARG_FN_TYPE)timeout, &tp);
 	tp.cache->down_load=i;
 	CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX);
 	}