294071 - secport.h comment confusion

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Jason Reid]

The file secport.h states that the interfaces within it are private. Yet
they are exposed in nss.def.

/*
 * secport.h - portability interfaces for security libraries
 *
 * This file abstracts out libc functionality that libsec depends on
 *
 * NOTE - These are not public interfaces

From nss.def:
PORT_SetUCS4_UTF8ConversionFunction;
PORT_SetUCS2_UTF8ConversionFunction;
PORT_SetUCS2_ASCIIConversionFunction;
PORT_UCS2_ASCIIConversion;
PORT_UCS2_UTF8Conversion;

If some of the functions are public, then the code comment should be altered to
reflect this.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

Update the comments in secport.h.  Remove dead code.

Does the new comment accurately describe the interfaces
declared in secport.h?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 183547 [details] [diff] [review]
Proposed patch



>- * This file abstracts out libc functionality that libsec depends on
>+ * This file abstracts out libc functionality that NSS depends on.

I suggest you just drop that line entirely.  
Very little of the stuff in this file has to do with libc functionality.  

>- * NOTE - These are not public interfaces
>+ * NOTE - Although these are public interfaces, they should in general
>+ * only be used inside NSS.  Some of these may be used outside NSS in
>+ * conjunction with NSS functions, for example, to free memory allocated
>+ * by NSS functions, to get error codes set by NSS functions, or to
>+ * create arena pools for use by NSS functions.

So, some of these functions are intended for use by any code that works
with NSS (such as code that allocates/destroys arenas), and others
were intended only for NSS internal use.  If the user of this file 
cannot tell which is which, maybe there's no point in saying they're 
not all intended for public use.  

But I don't want to force a reorganization of this file at this time.  
So, r=nelson

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Adding reviewers to cc list

Comment 4

[Robert Relyea]

Comment on attachment 183547 [details] [diff] [review]
Proposed patch

r+ = relyea
 I don't see much in there that would be dangerous to export. Most of the
functions applications need (free and allocate memory for use with NSS). If
they use it for their own application as well, I don't see a big issue.

bob

Comment 5

[Wan-Teh Chang]

Bob, Nelson, would you like me to just remove the
comments?

Comment 6

[Wan-Teh Chang]

Attachment: Proposed patch v2

Simply remove out-of-date comments.  May I check
this in on the trunk (NSS 3.10.1)?

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 183863 [details] [diff] [review]
Proposed patch v2

r=nelson

Comment 8

[Wan-Teh Chang]

Patch checked in on the NSS trunk for NSS 3.10.1.

Checking in secport.h;
/cvsroot/mozilla/security/nss/lib/util/secport.h,v  <--  secport.h
new revision: 1.12; previous revision: 1.11
done