Fix a particularly nasty bug in the hash tables, where an old would continue to get used when the value was replaced. Also a bunch of memory allocation tuning.

7 files changed, 14 insertions, 52 deletions

diff --git a/ckcapi-builtin.c b/ckcapi-builtin.c
index 21115a3..7bfc0a0 100644
--- a/ckcapi-builtin.c
+++ b/ckcapi-builtin.c
@@ -134,7 +134,7 @@ builtin_load_data(CkCapiSession* sess, CkCapiObject* obj, CkCapiObjectData** obj
 	ASSERT(objdata);
 	ASSERT(num_builtins > 0);
 
-	bdata = (BuiltinObjectData*)calloc(sizeof(BuiltinObjectData), 1);
+	bdata = (BuiltinObjectData*)calloc(1, sizeof(BuiltinObjectData));
 	if(!bdata)
 		return CKR_HOST_MEMORY;
 
@@ -181,7 +181,7 @@ register_builtin_object(CkCapiSession* sess, CK_ATTRIBUTE_PTR attr, CkCapiObject
 	BuiltinObject* bobj;
 	CK_RV ret;
 
-	bobj = calloc(sizeof(BuiltinObject), 1);
+	bobj = calloc(1, sizeof(BuiltinObject));
 	if(!bobj)
 		return CKR_HOST_MEMORY;
 
diff --git a/ckcapi-cert.c b/ckcapi-cert.c
index 4fe1d41..e7edde7 100644
--- a/ckcapi-cert.c
+++ b/ckcapi-cert.c
@@ -40,12 +40,6 @@ typedef struct _CertObject
 	/* Together these can uniquely identify a certificate */
 	CRYPT_INTEGER_BLOB serial;
 	CERT_NAME_BLOB issuer;
-
-	/* 
-	 * This must stay together. It comprises a unique 
-	 * key, together with the data that runs off the end.
-	 */
-	int otype;
 }
 CertObject;
 
@@ -85,7 +79,7 @@ parse_basic_constraints(CertObjectData* cdata, CK_ULONG* category)
 		return ckcapi_winerr_to_ckr(GetLastError());
 
 	/* Allocate enough memory */
-	basic = (CERT_BASIC_CONSTRAINTS_INFO*)calloc(size, 1);
+	basic = (CERT_BASIC_CONSTRAINTS_INFO*)calloc(1, size);
 	if(!basic)
 		return CKR_HOST_MEMORY;
 
@@ -294,7 +288,7 @@ cert_alloc_data(CkCapiSession* sess, CkCapiObject* obj, PCCERT_CONTEXT cert)
 {
 	CertObjectData* cdata;
 
-	cdata = (CertObjectData*)calloc(sizeof(CertObjectData), 1);
+	cdata = (CertObjectData*)calloc(1, sizeof(CertObjectData));
 	if(!cdata)
 		return NULL;
 	
@@ -596,15 +590,10 @@ register_cert_object(CkCapiSession* sess, PCCERT_CONTEXT cert, CkCapiObject** ob
 	len = cert->pCertInfo->SerialNumber.cbData + 
 		  cert->pCertInfo->Issuer.cbData;
 
-	/* Add one in case null termination is needed */
-	len++;
-
-	cobj = calloc(sizeof(CertObject) + len, 1);
+	cobj = calloc(1, sizeof(CertObject) + len);
 	if(!cobj)
 		return CKR_HOST_MEMORY;
 
-	cobj->otype = OBJECT_CERT;
-
 	cobj->obj.id = 0;
 	cobj->obj.obj_funcs = &cert_object_vtable;
 
diff --git a/ckcapi-key.c b/ckcapi-key.c
index 5cf60ba..abf37de 100644
--- a/ckcapi-key.c
+++ b/ckcapi-key.c
@@ -533,7 +533,7 @@ key_date_attribute(CkCapiObjectData* objdata, CK_ATTRIBUTE_PTR attr)
 	 */
 	case CKA_END_DATE:
 	case CKA_START_DATE:
-		return CKR_ATTRIBUTE_TYPE_INVALID;
+		return ckcapi_return_data(attr, "", 0);
 
 	default:
 		return CKR_ATTRIBUTE_TYPE_INVALID;
@@ -774,7 +774,7 @@ register_key_object(CkCapiSession* sess, CK_OBJECT_CLASS cls,
 	ASSERT(key_identifier);
 	ASSERT(cls == CKO_PRIVATE_KEY || cls == CKO_PUBLIC_KEY);
 
-	kobj = calloc(sizeof(KeyObject) + key_identifier->cbData, 1);
+	kobj = calloc(1, sizeof(KeyObject) + key_identifier->cbData);
 	if(!kobj)
 		return CKR_HOST_MEMORY;
 
diff --git a/ckcapi-object.h b/ckcapi-object.h
index 9a664d1..4ba5d09 100644
--- a/ckcapi-object.h
+++ b/ckcapi-object.h
@@ -98,32 +98,4 @@ CK_RV				ckcapi_object_data_get_attrs	(CkCapiObjectData* objdata, CK_ATTRIBUTE_P
 #define DBGOD(objdata, msg) \
 	ckcapi_debug("O%d: %s", (objdata) ? (objdata)->obj : 0, (msg))
 
-/* 
- * Each object has a unique key which guarantees that we're 
- * not loading the same objects over and over again. 
- * Usually these are contiguous members of a struct. These 
- * macros help calculate the address and length of such a 
- * unique key 
- */
-
-/* The unique key starts at the address of the starting struct member */
-#define UNIQUE_KEY_AT(obj, mem)	\
-	(void*)(&((obj->mem)))
-
-/* Calculates key length between first and last struct members */
-#define UNIQUE_KEY_LEN(obj, first, last) \
-	UNIQUE_KEY_VAR_LEN(obj, first, last, sizeof(obj->last))
-
-/* Calcs key len between first and a certain num of bytes past last struct member */
-#define UNIQUE_KEY_VAR_LEN(obj, first, last, len) \
-	((((char*)&((obj->last))) - ((char*)&((obj->first)))) + (len))
-
-/* Used internally to have a unique id for different object types */
-enum
-{
-	OBJECT_CERT =	 1,
-	OBJECT_BUILTIN = 2,
-	OBJECT_TRUST =	 3
-};
-
 #endif /* CKCAPI_OBJECT_H */
diff --git a/ckcapi-token.c b/ckcapi-token.c
index 0f93a45..c9b6f34 100644
--- a/ckcapi-token.c
+++ b/ckcapi-token.c
@@ -247,7 +247,6 @@ ckcapi_token_register_object(CK_SLOT_ID slot, CkCapiObject* obj)
 				{
 					ret = CKR_HOST_MEMORY;
 				}
-				
 			}
 			else
 			{
diff --git a/ckcapi-trust.c b/ckcapi-trust.c
index 3faa4b2..b048b9c 100644
--- a/ckcapi-trust.c
+++ b/ckcapi-trust.c
@@ -361,7 +361,7 @@ parse_restrictions(TrustObjectData* tdata)
 		return ckcapi_winerr_to_ckr(GetLastError());
 
 	/* Allocate enough memory */
-	rst = (CRYPT_BIT_BLOB*)calloc(size, 1);
+	rst = (CRYPT_BIT_BLOB*)calloc(1, size);
 	if(!rst)
 		return CKR_HOST_MEMORY;
 
@@ -460,7 +460,7 @@ register_trust_object(CkCapiSession* sess, CkCapiObject* cert, CkCapiObject** ob
 	TrustObject* tobj;
 	CK_RV ret;
 
-	tobj = calloc(sizeof(TrustObject), 1);
+	tobj = calloc(1, sizeof(TrustObject));
 	if(!tobj)
 		return CKR_HOST_MEMORY;
 
diff --git a/ckcapi-util.c b/ckcapi-util.c
index 80e1b40..ae8c39b 100644
--- a/ckcapi-util.c
+++ b/ckcapi-util.c
@@ -296,7 +296,7 @@ equal_default(const void* a, const void* b)
 static HashEntry** 
 alloc_array(CkCapiHash* ht, size_t max)
 {
-	return calloc(sizeof(*(ht->array)) * (max + 1), 1);
+	return calloc(1, sizeof(*(ht->array)) * (max + 1));
 }
 
 CkCapiHash* 
@@ -352,6 +352,7 @@ expand_array(CkCapiHash* ht)
 	HashEntry** new_array;
 	size_t new_max;
 	HashEntry* he;
+	HashEntry* next;
 	size_t i;
 
 	new_max = ht->max * 2 + 1;
@@ -362,7 +363,8 @@ expand_array(CkCapiHash* ht)
 		
 	for(i = 0; i <= ht->max; ++i)
 	{
-		for(he = ht->array[i]; he; he = he->next)
+		for(he = ht->array[i], next = he ? he->next : NULL; 
+			he != NULL; he = next, next = next ? next->next : NULL)
 		{
 			unsigned int j = he->hash & new_max;
 			he->next = new_array[j];
@@ -413,7 +415,6 @@ find_entry(CkCapiHash* ht, const void* key, void* val)
 	{
 		/* Key points to external data */
 		he->key = key;
-
 		he->next = NULL;
 		he->hash = hash;
 		he->val	= val;
@@ -442,6 +443,7 @@ ckcapi_hash_set(CkCapiHash* ht, const void* key, void* val)
 	if(hep && *hep) 
 	{
 		/* replace entry */
+		(*hep)->key = key;
 		(*hep)->val = val;
 			
 		/* check that the collision rate isn't too high */