302152 - Lots and lots of compiler warnings fixed

Status: REOPENED

(NSPR :: NSPR, defect)

Description

[Mikhail T.]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.4; FreeBSD; X11; amd64) KHTML/3.4.1 (like Gecko)
Build Identifier: 

Compiling with ``-Wall -Wno-parentheses'' raises and awful lot of warnings 
(especially on amd64), which makes searching for genuine problems very 
difficult. 
 
Most of the problems fall into one of the following categories: 
1. mismatch between arguments and format string of printf functions. The main 
culprits are using various integer formats to print pointers (instead of simple 
%p) and using long-specifiers (%ld) for 32-bit values. On both 32-bit and 
64-bit int is usually a 32-bit value, so simply removing the 'l' seems enough. 
To be absolutely safe, of course, one should use %ld and cast each variable to 
long. My patch does not do that. 
2. Casting pointers to integers of different (smaller) size. My patch casts to 
intptr_t first -- I examined all such code and found no cases, where actual 
loss of bits were possible. 
3. Unused vairables, static functions -- removed or ifdef-ed out. 
 
Having spent much time in this code, I must complain, that it overuses NSPR's 
own types (such as PRInt32). In simple cases -- when no assumptions need to be 
made regarding sizeof -- standard C types (such as int) should be used. The 
patch for lib/tests/string.c is particularly large because of this habit, for 
example. 

Reproducible: Always

Comment 1

[Mikhail T.]

Attachment: patch for library itself -- no real problems, just cleanup

Comment 2

[Mikhail T.]

Attachment: patch for the lib/tests files -- lots of printf-format warnings fixed

Comment 3

[Mikhail T.]

Attachment: patches for the pr/tests

The last patch in this file is for the runtests.sh script itself. It changes
the script to show the output of the failed tests to stdout for the purposes of
external testing (such as by FreeBSD port).

Please, consider the diffs for the .c files first...

Comment 4

[Mikhail T.]

BTW, is there a "test harness" somewhere for the lib/tests? The string and 
base64t both seem to do something meaningful by themselves, but arena requires 
command line arguments... Can you give me a reasonable example of such? Thanks! 

Comment 5

[Chris Peterson [:cpeterson]]

These NSPR patches from 2005 are no longer relevant. NSPR warnings should be fixed upstream.

Comment 7

[Mikhail T.]

(In reply to Chris Peterson [:cpeterson] from comment #5)
> These NSPR patches from 2005 are no longer relevant.

Unfortunately, NSPR still generates TONNES of compiler-warnings today, 10 years later... Here are the up-to-date patches as used by the FreeBSD port. The patches aren't FreeBSD-specific and ought to be merged into release:

https://svnweb.freebsd.org/ports/head/devel/nspr/files/patch-warnings?view=log
https://svnweb.freebsd.org/ports/head/devel/nspr/files/patch-tests?view=log
https://svnweb.freebsd.org/ports/head/devel/nspr/files/patch-libtests?view=log

> NSPR warnings should be fixed upstream.

What is "upstream" here? Is NSPR no longer a Mozilla product?

Comment 8

[Mikhail T.]

(In reply to Chris Peterson [:cpeterson] from comment #7 to Bug 301985)

> NSPR and NSS are maintained in a CVS repo shared by some non-Mozilla
> projects, independent of Firefox code.

But they still use bugzilla.mozilla.org for issue-tracking, do they not?

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.