Bugzilla bug 132461: changes suggested by Bob Relyea after his code review:

1. Add SECMOD_LoadModule back to nss.def.
2. Back out the SECMOD_AddModule/AddModuleEx changes.
I also updated the comments and made some changes to emulate the
indentation and comment styles of the original code.
Modifies files: nss/nss.def pk11wrap/pk11pars.c pk11wrap/pk11util.c

security/nss/lib/nss/nss.def

@@ -564,6 +564,7 @@ SECMOD_AddNewModuleEx;
 SECMOD_DeleteModule;
 SECMOD_FreeModuleSpecList;
 SECMOD_GetModuleSpecList;
+SECMOD_LoadModule;
 SECMOD_LoadUserModule;
 SECMOD_UnloadUserModule;
 SECMOD_UpdateModule;

security/nss/lib/pk11wrap/pk11pars.c

@@ -272,9 +272,9 @@ SECMOD_FreeModuleSpecList(SECMODModule *parent, char **moduleSpecList)
     return SECSuccess;
 }
 
-/* internal function that loads a PKCS#11 module but does not add it to the
-   default NSS trust domain */
-
+/*
+ * load a PKCS#11 module but do not add it to the default NSS trust domain
+ */
 SECMODModule *
 SECMOD_LoadModule(char *modulespec,SECMODModule *parent, PRBool recurse)
 {
@@ -353,19 +353,17 @@ loser:
     return module;
 }
 
-/* exported function that loads a PKCS#11 module and adds it to the default
-   NSS trust domain */
-
+/*
+ * load a PKCS#11 module and add it to the default NSS trust domain
+ */
 SECMODModule *
 SECMOD_LoadUserModule(char *modulespec,SECMODModule *parent, PRBool recurse)
 {
     SECStatus rv = SECSuccess;
     SECMODModule * newmod = SECMOD_LoadModule(modulespec, parent, recurse);
-    if (newmod)
-    {
+    if (newmod) {
         rv = STAN_AddModuleToDefaultTrustDomain(newmod);
-        if (SECSuccess != rv)
-        {
+        if (SECSuccess != rv) {
             SECMOD_DestroyModule(newmod);
             return NULL;
         }
@@ -373,20 +371,19 @@ SECMOD_LoadUserModule(char *modulespec,SECMODModule *parent, PRBool recurse)
     return newmod;
 }
 
-/* exported call that removes the PKCS#11 module from the default NSS trust
-   domain, call C_Finalize, and destroy the module structure */
-
+/*
+ * remove the PKCS#11 module from the default NSS trust domain, call
+ * C_Finalize, and destroy the module structure
+ */
 SECStatus SECMOD_UnloadUserModule(SECMODModule *mod)
 {
     SECStatus rv = SECSuccess;
     int atype = 0;
-    if (!mod)
-    {
+    if (!mod) {
         return SECFailure;
     }
     rv = STAN_RemoveModuleFromDefaultTrustDomain(mod);
-    if (SECSuccess != rv)
-    {
+    if (SECSuccess != rv) {
         return SECFailure;
     }
     return SECMOD_DeleteModuleEx(NULL, mod, &atype, PR_FALSE);

security/nss/lib/pk11wrap/pk11util.c

@@ -224,13 +224,13 @@ PK11SlotInfo *SECMOD_LookupSlot(SECMODModuleID moduleID,CK_SLOT_ID slotID) {
     return NULL;
 }
 
-/*
- * find a module by name or module pointer, and delete it off the module list
- * optionally remove it from secmod.db
- */
 
+/*
+ * find a module by name or module pointer and delete it off the module list.
+ * optionally remove it from secmod.db.
+ */
 SECStatus
-SECMOD_DeleteModuleEx(char * name, SECMODModule *mod, int *type, PRBool permdb) {
+SECMOD_DeleteModuleEx(char *name, SECMODModule *mod, int *type, PRBool permdb) {
     SECMODModuleList *mlp;
     SECMODModuleList **mlpp;
     SECStatus rv = SECFailure;
@@ -241,8 +241,8 @@ SECMOD_DeleteModuleEx(char * name, SECMODModule *mod, int *type, PRBool permdb)
     SECMOD_GetWriteLock(moduleLock);
     for(mlpp = &modules,mlp = modules; 
 				mlp != NULL; mlpp = &mlp->next, mlp = *mlpp) {
-        if ( ( name && (PORT_Strcmp(name,mlp->module->commonName) == 0) ) ||
-             mod == mlp->module ) {
+	if ((name && (PORT_Strcmp(name,mlp->module->commonName) == 0)) ||
+							mod == mlp->module) {
 	    /* don't delete the internal module */
 	    if (!mlp->module->internal) {
 		SECMOD_RemoveList(mlpp,mlp);
@@ -258,17 +258,18 @@ SECMOD_DeleteModuleEx(char * name, SECMODModule *mod, int *type, PRBool permdb)
     }
     SECMOD_ReleaseWriteLock(moduleLock);
 
+
     if (rv == SECSuccess) {
-        if (permdb) {
+	if (permdb) {
  	    SECMOD_DeletePermDB(mlp->module);
-        }
+	}
 	SECMOD_DestroyModuleListElement(mlp);
     }
     return rv;
 }
 
 /*
- * find a module by name and delete it of the module list
+ * find a module by name and delete it off the module list
  */
 SECStatus
 SECMOD_DeleteModule(char *name, int *type) {
@@ -276,7 +277,7 @@ SECMOD_DeleteModule(char *name, int *type) {
 }
 
 /*
- * find a module by name and delete it of the module list
+ * find a module by name and delete it off the module list
  */
 SECStatus
 SECMOD_DeleteInternalModule(char *name) {
@@ -340,7 +341,7 @@ SECMOD_DeleteInternalModule(char *name) {
 }
 
 SECStatus
-SECMOD_AddModuleEx(SECMODModule *newModule, PRBool permdb) {
+SECMOD_AddModule(SECMODModule *newModule) {
     SECStatus rv;
     SECMODModule *oldModule;
 
@@ -364,9 +365,7 @@ SECMOD_AddModuleEx(SECMODModule *newModule, PRBool permdb) {
 	newModule->parent = SECMOD_ReferenceModule(defaultDBModule);
     }
 
-    if (permdb) {
-        SECMOD_AddPermDB(newModule);
-    }
+    SECMOD_AddPermDB(newModule);
     SECMOD_AddModuleToList(newModule);
 
     rv = STAN_AddModuleToDefaultTrustDomain(newModule);
@@ -374,11 +373,6 @@ SECMOD_AddModuleEx(SECMODModule *newModule, PRBool permdb) {
     return rv;
 }
 
-SECStatus
-SECMOD_AddModule(SECMODModule *newModule) {
-    return SECMOD_AddModuleEx(newModule, PR_TRUE);
-}
-
 PK11SlotInfo *SECMOD_FindSlot(SECMODModule *module,char *name) {
     int i;
     char *string;