505561 - Need a generic function a la SECMOD_OpenUserDB() that can be used on non-softoken modules.

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Robert Relyea]

Currently, softoken has the ability to load and unload new databases on the fly. This is accomplished through create calls using a NSS specific object, passing the configuration parameters that would have been passed at init time. There is no reason other tokens could not also use this mechanism, but there needs to be a function which can export that functionality to the user.

Comment 1

[Robert Relyea]

Attachment: Create new function that will attempt to open new slots on any token, not just softoken

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 390021 [details] [diff] [review]
Create new function that will attempt to open new slots on any token, not just softoken

I believe this patch is correct, so r+.
However, there are lots of comments that must be fixed.
I trust you will fix them before committing.  
If you want, you can ask me to review it again.

>+ * This is exactly the same as OpenUserDB except it can be called on only
>+ * module that understands softoken style new slot entries. The resulting
>+ * slot can be closed using SECMOD_CloseUserDB above. Value of moduleSpec
>+ * is token specific.

That says "can be called on only module". :( 
Maybe "can only be called on a module" ?

Perhaps the comment should explain how the caller can and should determine 
if a module understands softoken-style new slot entries. 
The comment should definitely explain what will happen if this new function 
is called on a module that does NOT support them.

>+ * Fine a unused slot id in module. If 'usePrime' is set to true allow
>+ * selection of the primary slot.
>+ */
>+static CK_SLOT_ID
>+secmod_FindFreeSlot(SECMODModule *mod)

The comment above mentions "usePrime", but there is no such variable or 
argument.

>+/*
>+ * Attempt to open a new slot on a new slot.
>+ *
>+ * This works the same os OpenUserDB except it can be called against
>+ * Any module, not just the internal one.
>+ * 
>+ * NewSlots can be closed with SECMOD_CloseUserDB();
>+ *
>+ * Modespec are slot dependent.

Modespec?  I think you meant ModuleSpec

>+    if (sendSpec == NULL) {
>+	/* PR_smprintf does not set no memory error */

That sounds like a double-negative.  It makes more sense (and is better 
grammar) if you change it to:   "does not set NO_MEMORY error"

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 390021 [details] [diff] [review]
Create new function that will attempt to open new slots on any token, not just softoken

Oh, one more bad comment needs to be fixed.

>+/*
>+ * Attempt to open a new slot on a new slot.

A new slot on a new slot?  uh ... what?

Comment 4

[Robert Relyea]

> That says "can be called on only module". :( 
> Maybe "can only be called on a module" ?
 
Yes.

If the module does not support the protocol, it will reject the create object command and this function will return NULL (rather than a new slot).

> The comment above mentions "usePrime", but there is no such variable or 
> argument.

I'll remove it. It turned out to be unnecessary.

> Modespec?  I think you meant ModuleSpec

yes

> That sounds like a double-negative.

I'll use the actuall SEC_ERR that it doesn't set...;).

> A new slot on a new slot?  uh ... what?

I'll change it to "Attempt to open a new slot"...;).

bob

Comment 5

[Robert Relyea]

(+128/72) Lines changed.
When 	Who 	File 	Rev 	+/- 	Description
2009-07-29 17:29	rrelyea%redhat.com 	mozilla/security/nss/lib/nss/nss.def 	1.199 	6/0  	

r=nelsonb
Bug 505561 - Need a generic function a la SECMOD_OpenUserDB() that can be used on non-softoken modules.
2009-07-29 17:29	rrelyea%redhat.com 	mozilla/security/nss/lib/pk11wrap/pk11pub.h 	1.29 	9/0 
2009-07-29 17:29	rrelyea%redhat.com 	mozilla/security/nss/lib/pk11wrap/pk11util.c 	1.55 	113/72