486405 - Allocator mismatches in pk12util.c

Status: RESOLVED FIXED

(NSS :: Tools, defect, P3)

Description

[[e-mail]]

p12u_DestroyContext uses PR_Free((*ppCtx)->filename); but
p12u_InitContext uses p12ctx->filename = strdup(filename);

Either p12u_DestroyContext could be changed to use free() or they could both be switched e.g. to PL_strdup and PL_strfree.

main uses PORT_ZFree(p12FilePw.data, PL_strlen(p12FilePw.data)); but earlier allocated it using PL_strdup. I understand PORT_ZFree is a secure delete, otherwise I would have simply suggested switching to PL_strfree.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: Patch v1 for NSS Trunk

This patch works, in that it passes our usual NSS tests.
I believe it fixes the allocator mismatch problems.

Neil, if it is convenient for you to do so, please test this patch 
with your allocator mismatch testing apparatus.  Thanks.

Comment 2

[[e-mail]]

Comment on attachment 370705 [details] [diff] [review]
Patch v1 for NSS Trunk

This patch successfully passes my allocator mismatch detection.

Comment 3

[[e-mail]]

Comment on attachment 370705 [details] [diff] [review]
Patch v1 for NSS Trunk

>+    if (slot) 
>+    	PK11_FreeSlot(slot);
Note: I don't know what spacing guide you use, but as the git-apply utility points out, the first of these two lines ends with a space, while the second contains four spaces followed by a tab.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

(In reply to comment #3)
> Note: I don't know what spacing guide you use, but as the git-apply utility
> points out, the first of these two lines ends with a space, while the second
> contains four spaces followed by a tab.

NSS spacing guidelines are published somehwere, but here is a short summary
- we don't care about trailing spaces
- our tab stops are set every 8 columns
- our "indentation quantum" is 4 columns.  

We allow any combination of tabs and spaces to be used to achieve proper indentation (provided that it looks right when tabs are set to 8 columns).  
Although it is slightly preferrable to not preceed tabs with spaces, as long 
as it looks right...
So, the above observations do not indicate unacceptable code for NSS.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Checking in pk12util.c; new revision: 1.41; previous revision: 1.40

Comment 6

[Julien Pierre]

I wonder if this is in any way related to bug 486811 . The line numbers for the patch don't match the other stacks though.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

I'd guess we have other allocator mismatches that we have not yet uncovered.