[PATCH 3/3] Add caching for PK11_GetTokenUri() results

From: David Woodhouse <[email protected]>

Is this really worth the complexity? It's a *lot* of complexity on
the providing side, to remove a *small* amount of complexity (having
to free the string) on the calling side. And it loses the flexibility
of being able to specify the URI type too.
---
 cmd/certutil/certutil.c |  4 +---
 lib/pk11wrap/pk11pub.h  |  3 +--
 lib/pk11wrap/pk11slot.c | 36 ++++++++++++++++++++++++++++++++----
 lib/pk11wrap/secmodti.h |  1 +
 4 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/cmd/certutil/certutil.c b/cmd/certutil/certutil.c
index 06bc45b..9d7a563 100644
--- a/cmd/certutil/certutil.c
+++ b/cmd/certutil/certutil.c
@@ -990,12 +990,10 @@ ListModules(void)
 
     /* look at each slot*/
     for (le = list->head; le; le = le->next) {
-        char *token_uri = PK11_GetTokenUri(le->slot, P11URI_FOR_TOKEN);
         printf("\n");
         printf("    slot: %s\n", PK11_GetSlotName(le->slot));
         printf("   token: %s\n", PK11_GetTokenName(le->slot));
-        printf("     uri: %s\n", token_uri);
-        P11URI_FreeString(token_uri);
+        printf("     uri: %s\n", PK11_GetTokenUri(le->slot));
     }
     PK11_FreeSlotList(list);
 
diff --git a/lib/pk11wrap/pk11pub.h b/lib/pk11wrap/pk11pub.h
index e496b91..6663649 100644
--- a/lib/pk11wrap/pk11pub.h
+++ b/lib/pk11wrap/pk11pub.h
@@ -14,7 +14,6 @@
 #include "seccomon.h"
 #include "pkcs7t.h"
 #include "cmsreclist.h"
-#include "p11uri.h"
 
 /*
  * Exported PK11 wrap functions.
@@ -79,7 +78,7 @@ PRBool PK11_IsReadOnly(PK11SlotInfo *slot);
 PRBool PK11_IsInternal(PK11SlotInfo *slot);
 PRBool PK11_IsInternalKeySlot(PK11SlotInfo *slot);
 char * PK11_GetTokenName(PK11SlotInfo *slot);
-char * PK11_GetTokenUri(PK11SlotInfo *slot, P11URIType type);
+char * PK11_GetTokenUri(PK11SlotInfo *slot);
 char * PK11_GetSlotName(PK11SlotInfo *slot);
 PRBool PK11_NeedLogin(PK11SlotInfo *slot);
 PRBool PK11_IsFriendly(PK11SlotInfo *slot);
diff --git a/lib/pk11wrap/pk11slot.c b/lib/pk11wrap/pk11slot.c
index 1b42bd5..aade503 100644
--- a/lib/pk11wrap/pk11slot.c
+++ b/lib/pk11wrap/pk11slot.c
@@ -20,7 +20,6 @@
 #include "utilpars.h"
 #include "p11uri.h"
 
-
 /*************************************************************
  * local static and global data
  *************************************************************/
@@ -394,6 +393,7 @@ PK11_NewSlotInfo(SECMODModule *mod)
     slot->cert_count = 0;
     slot->slot_name[0] = 0;
     slot->token_name[0] = 0;
+    slot->token_uri = NULL;
     PORT_Memset(slot->serial,' ',sizeof(slot->serial));
     slot->module = NULL;
     slot->authTransact = 0;
@@ -442,7 +442,9 @@ PK11_DestroySlot(PK11SlotInfo *slot)
    if (slot->module) {
 	SECMOD_SlotDestroyModule(slot->module,PR_TRUE);
    }
-
+   if (slot->token_uri) {
+       P11URI_FreeString(slot->token_uri);
+   }
    /* ok, well not quit finally... now we free the memory */
    PORT_Free(slot);
 }
@@ -1121,6 +1123,7 @@ PK11_InitToken(PK11SlotInfo *slot, PRBool loadCerts)
     slot->series++; /* allow other objects to detect that the 
 		      * slot is different */
     slot->flags = tokenInfo.flags;
+    slot->token_uri = NULL;
     slot->needLogin = ((tokenInfo.flags & CKF_LOGIN_REQUIRED) ? 
 							PR_TRUE : PR_FALSE);
     slot->readOnly = ((tokenInfo.flags & CKF_WRITE_PROTECTED) ? 
@@ -1299,6 +1302,10 @@ PK11_TokenRefresh(PK11SlotInfo *slot)
     }
 
     slot->flags = tokenInfo.flags;
+    if (slot->token_uri) {
+	P11URI_FreeString(slot->token_uri);
+	slot->token_uri = NULL;
+    }
     slot->needLogin = ((tokenInfo.flags & CKF_LOGIN_REQUIRED) ? 
 							PR_TRUE : PR_FALSE);
     slot->readOnly = ((tokenInfo.flags & CKF_WRITE_PROTECTED) ? 
@@ -1605,10 +1612,17 @@ PK11_GetTokenName(PK11SlotInfo *slot)
 }
 
 char *
-PK11_GetTokenUri(PK11SlotInfo *slot, P11URIType type)
+PK11_GetTokenUri(PK11SlotInfo *slot)
 {
     P11URI *uri;
 
+    PK11_EnterSlotMonitor(slot);
+    if (slot->token_uri)
+	goto out;
+
+    /* Have to unlock to call PK11_GetTokenInfo */
+    PK11_ExitSlotMonitor(slot);
+
     uri = P11URI_New();
     if (!uri)
         return NULL;
@@ -1618,7 +1632,16 @@ PK11_GetTokenUri(PK11SlotInfo *slot, P11URIType type)
 	return NULL;
     }
 
-    return P11URI_Format(uri, type);
+    PK11_EnterSlotMonitor(slot);
+
+    /* Another thread may have beaten us to it */
+    if (slot->token_uri == NULL)
+	slot->token_uri = P11URI_Format(uri, P11URI_FOR_TOKEN);
+
+    P11URI_Free(uri);
+ out:
+    PK11_ExitSlotMonitor(slot);
+    return slot->token_uri;
 }
 
 char *
@@ -2342,6 +2365,11 @@ PK11_ResetToken(PK11SlotInfo *slot, char *sso_pwd)
     /* initialize the token */    
     PK11_EnterSlotMonitor(slot);
 
+    if (slot->token_uri) {
+	P11URI_FreeString(slot->token_uri);
+	slot->token_uri = NULL;
+    }
+
     /* first shutdown the token. Existing sessions will get closed here */
     PK11_GETTAB(slot)->C_CloseAllSessions(slot->slotID);
     slot->session = CK_INVALID_SESSION;
diff --git a/lib/pk11wrap/secmodti.h b/lib/pk11wrap/secmodti.h
index 2b63130..27a774a 100644
--- a/lib/pk11wrap/secmodti.h
+++ b/lib/pk11wrap/secmodti.h
@@ -98,6 +98,7 @@ struct PK11SlotInfoStr {
      * allow them to become null terminated strings */
     char slot_name[65];
     char token_name[33];
+    char *token_uri; /* cached result of P11URI_Format() */
     PRBool hasRootCerts;
     PRBool hasRootTrust;
     PRBool hasRSAInfo;
-- 
2.7.4

-- 
dev-tech-crypto mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-crypto