326315 - Warning Reduction bug for NSS 3.11.1 + NSS 3.12

Status: RESOLVED WORKSFORME

(NSS :: Build, defect, P3)

Description

[Nelson Bolyard (seldom reads bugmail)]

This bug is about excessive warnings in NSS source code.
This bug provides a place for any NSS developer to attach a patch that 
eliminates warnings, and get them reviewed, without having to create a 
new bug each time.  I will shortly attach the first warning reduction patch.

I propose that we'll mark this resolved/fixed when we make the 3.12 release.
I'd hope we can avoid having separate bugs for 3.11.1 and 3.12 about this.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: eliminate excessive warnings in two files (checked in)

Please review this patch using bugzilla's diff feature, because column alignment
will be correct in that view (IINM).

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 211067 [details] [diff] [review]
eliminate excessive warnings in two files (checked in)

Julien, please review.

Comment 3

[Julien Pierre]

Comment on attachment 211067 [details] [diff] [review]
eliminate excessive warnings in two files (checked in)

Looks fine, except it appears you only changed the identation for the nss_cmsrecipientinfo_create declaration, which is unnecessary.

Comment 4

[Wan-Teh Chang]

On the NSS_3_11_BRANCH, we should only fix serious warnings
such as undeclared functions (also called implicit declarations)
and uninitialized variables.

When tracking down regressions, we often need to read diffs.
People outside our team also evaluate the risk of an NSS
upgrade by reading diffs and tend to equate risk to the size
of the diffs.  So it's important to miminize diffs.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Checked in on trunk only.

Checking in certdb/genname.c;   new revision: 1.30; previous revision: 1.29
Checking in smime/cmsrecinfo.c; new revision: 1.17; previous revision: 1.16

Leaving bug open until we ship NSS 3.12

Comment 6

[timeless]

Comment on attachment 211067 [details] [diff] [review]
eliminate excessive warnings in two files (checked in)

>-	    PL_strncasecmp(name->data, constraint->data, constraint->len))
>+	    PL_strncasecmp((char *)name->data, 
>+			   (char *)constraint->data, constraint->len))

This is wrong.

mozilla/nsprpub/lib/libc/include/plstr.h 

 258 PR_EXTERN(PRIntn)
 259 PL_strncasecmp(const char *a, const char *b, PRUint32 max);

you're casting away const for no reason. the function takes const's.

I was about to file this bug because the sun compiler doesn't appreciate the uncasted version. My patch use (const char*).

And yes, I know that C doesn't properly decorate such things, but if you're going to fix a warning, you should fix it with the safest proper cast even if the callee promises not to hurt the data, you shouldn't temporarily claim that the data is mutable.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

timeless, feel free to submit a corrective patch.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Unsetting target milestone in unresolved bugs whose targets have passed.

Comment 9

[Chris Peterson [:cpeterson]]

These NSPR warnings have since been fixed. \o/