registry team mailing list archive
-
registry team
-
Mailing list archive
-
Message #06557
[Bug 180179] Re: [PATCH] Fix nspr warnings with -Wstrict-prototypes
Launchpad has imported 23 comments from the remote bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=410677.
If you reply to an imported comment from within Launchpad, your comment
will be sent to the remote bug automatically. Read more about
Launchpad's inter-bugtracker facilities at
https://help.launchpad.net/InterBugTracking.
------------------------------------------------------------------------
On 2008-01-03T12:09:45+00:00 Richard Laager wrote:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en; rv:1.8.1.11) Gecko/20071204 Epiphany/2.20 Firefox/2.0.0.11
Build Identifier: libnspr4-dev 4.6.6-3 from Ubuntu
I compiled Pidgin with -Wstrict-prototypes and got some warnings from
NSPR. Attached is a patch to fix those warnings. I haven't done any more
looking into NSPR to find other instances of this problem, so I can't
say for sure if this is it or not.
Reproducible: Always
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/0
------------------------------------------------------------------------
On 2008-01-03T12:10:20+00:00 Richard Laager wrote:
Created an attachment (id=295261)
A proposed patch to fix this.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/1
------------------------------------------------------------------------
On 2008-01-03T12:53:57+00:00 Wan-Teh Chang wrote:
Thanks for the bug report and the patch.
Since prlink.h is a public header file and this changes two typedefs, it may break
source compatibility, so I'm afraid that we can't take this patch. Is it possible to
tell gcc to ignore strict prototype warnings about these two typedefs?
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/2
------------------------------------------------------------------------
On 2008-01-03T13:13:44+00:00 Richard Laager wrote:
(In reply to comment #2)
> Thanks for the bug report and the patch.
>
> Since prlink.h is a public header file and this changes two typedefs, it may
> break
> source compatibility, so I'm afraid that we can't take this patch.
Are these functions supposed to take arguments or not? In other words,
what is the correct prototype here?
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/3
------------------------------------------------------------------------
On 2008-07-07T02:20:51+00:00 SharkCZ wrote:
The same bug has been filled for Fedora/RHEL - https://bugzilla.redhat.com/show_bug.cgi?id=451616
So is there any chance for a solution?
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/8
------------------------------------------------------------------------
On 2008-07-08T09:27:46+00:00 Kaie wrote:
If I understand correctly, the type PRFuncPtr is supposed to work with
ANY function signature.
The functions that use this function-pointer type appear to operate on
generic functions, which have any signature.
I conclude the proposed patch is wrong, as it would only work with
functions that have a specific function signature: "empty parameter
list".
Is my understanding right or wrong?
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/9
------------------------------------------------------------------------
On 2008-07-08T09:54:37+00:00 Wan-Teh Chang wrote:
The type PRFuncPtr is like the void * pointer for function pointers.
PRFuncPtr is intended to be a universal function pointer type.
It should be cast to a specific function pointer type before calling
the function. So the function signature of PRFuncPtr doesn't matter.
I'm worried that the proposed change may break the compilation of
existing source code. Here is an example I made up:
PRBool foo(PRLibrary *lib, const char *name)
{
/*
* Instead of
* PRFuncPtr func_ptr;
* we declare the variable like this:
*/
void (*func_ptr)();
func_ptr = PR_FindFunctionSymbol(lib, name);
return func_ptr != NULL;
}
I'm worried that changing the definition of PRFuncPtr
would make the compilation fail. We should test the
above code on all the platforms we support. I've tested
GCC on Linux and the code still compiles.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/10
------------------------------------------------------------------------
On 2008-07-08T10:04:12+00:00 Richard Laager wrote:
(In reply to comment #5)
> If I understand correctly, the type PRFuncPtr is supposed to work with ANY
> function signature.
>
> The functions that use this function-pointer type appear to operate on generic
> functions, which have any signature.
>
> I conclude the proposed patch is wrong, as it would only work with functions
> that have a specific function signature: "empty parameter list".
This seems to be the case. Why not just use void * instead? This is
similar to what glib does (FYI, gpointer === void * and they're using an
output parameter rather than returning it):
http://library.gnome.org/devel/glib/stable/glib-Dynamic-Loading-of-
Modules.html#g-module-symbol
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/11
------------------------------------------------------------------------
On 2008-07-08T10:22:31+00:00 Wan-Teh Chang wrote:
Any pointer to an object may be converted to void *.
But a function pointer is not an object pointer. So
for portability we want to use a generic function pointer
type rather than void *.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/12
------------------------------------------------------------------------
On 2008-07-08T13:54:18+00:00 Nelson-bolyard wrote:
I recommend WONTFIX.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/13
------------------------------------------------------------------------
On 2008-07-09T01:08:35+00:00 SharkCZ wrote:
I think better would be first to know an opinion from some GCC or C
standard people what is the best practice in such situation.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/14
------------------------------------------------------------------------
On 2008-07-09T10:07:16+00:00 Wan-Teh Chang wrote:
We have several options.
1. Change the implementation of the GCC -Wstrict-prototypes
to only warn about actual invocation of a function declared
without specifying the argument types. That is, the mere
declaration of a function without specifying the argument
types should not cause a warning. Only the act of calling
such a function should be warned.
This is to recognize the existence of code that defines a
generic function pointer type like this:
typedef int (*func_ptr_t)();
or this:
typedef void (*func_ptr_t)();
2. Do not use -Wstrict-prototypes on files that include "prlink.h".
3. Ignore the -Wstrict-prototypes warnings in "prlink.h".
4. Change "prlink.h" to suppress this warning. I think
#pragma GCC diagnostic ignored "-Wstrict-prototypes"
should work. But I don't know how to make it apply to
only certain lines in "prlink.h", and restore to the *original
setting*.
5. Change the definition of PRFuncPtr in "prlink.h". As
I explained before, NSPR needs to be backward compatible
(at both source and binary levels), which means existing
code should continue to compile against new versions of
NSPR headers. This is why I have to perform a lot of
experiments (GCC, Visual C++, Sun Studio, HP C, and IBM
C compilers) to see if changing the definition of
PRFuncPtr breaks the compilation or generates compiler
warnings on the sample code I gave in comment 6.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/15
------------------------------------------------------------------------
On 2009-09-06T14:43:33+00:00 Gk-gknw wrote:
Folks, this bug is now 20 months old, and still no solution to this.
This is really bad practice to just ignore such bug reports since we deal here with a security-related library, and you make folks more and more accept compiler warnings which seem to be fixable from your side. When I f.e. compile a project like libcurl with NSS support these warnings in prlink.h add up to ~100 times! And disabling the warnings with -Wno-strict-prototypes is really not a solution.
BTW. I found some more places where I get similar warnings:
--- cert.h.orig 2009-07-01 01:05:24.000000000 +0200
+++ cert.h 2009-09-06 18:11:04.000000000 +0200
@@ -1610,25 +1610,25 @@
* Returns a pointer to a static structure.
*/
extern const CERTRevocationFlags*
-CERT_GetPKIXVerifyNistRevocationPolicy();
+CERT_GetPKIXVerifyNistRevocationPolicy(void);
/*
* Returns a pointer to a static structure.
*/
extern const CERTRevocationFlags*
-CERT_GetClassicOCSPEnabledSoftFailurePolicy();
+CERT_GetClassicOCSPEnabledSoftFailurePolicy(void);
/*
* Returns a pointer to a static structure.
*/
extern const CERTRevocationFlags*
-CERT_GetClassicOCSPEnabledHardFailurePolicy();
+CERT_GetClassicOCSPEnabledHardFailurePolicy(void);
/*
* Returns a pointer to a static structure.
*/
extern const CERTRevocationFlags*
-CERT_GetClassicOCSPDisabledPolicy();
+CERT_GetClassicOCSPDisabledPolicy(void);
/*
* Verify a Cert with libpkix
@@ -1662,7 +1662,7 @@
/* The function return PR_TRUE if cert validation should use
* libpkix cert validation engine. */
-PRBool CERT_GetUsePKIXForValidation();
+PRBool CERT_GetUsePKIXForValidation(void);
SEC_END_PROTOS
I think you should just apply the void fix, and as time passes by we
will see if others will report new problems with the fixes (which I
doubt).
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/19
------------------------------------------------------------------------
On 2009-09-06T16:02:53+00:00 Gk-gknw wrote:
Hi Wan-Teh Chang,
(In reply to comment #11)
> We have several options.
> 4. Change "prlink.h" to suppress this warning. I think
>
> #pragma GCC diagnostic ignored "-Wstrict-prototypes"
>
> should work. But I don't know how to make it apply to
> only certain lines in "prlink.h", and restore to the *original
> setting*.
http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html
this works for me:
#pragma GCC diagnostic ignored "-Wstrict-prototypes"
#include <nspr.h>
#pragma GCC diagnostic warning "-Wstrict-prototypes"
I have tested this pragma with lowest gcc version 4.2.1 - a test with
4.0.2 shows that the pragma is not understood. Will attach a patch
against prlink.h with the proper ifdefs surrounded.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/20
------------------------------------------------------------------------
On 2009-09-06T16:04:46+00:00 Gk-gknw wrote:
Created an attachment (id=398986)
new patch against prlink.h to avoid warnings with pragmas
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/21
------------------------------------------------------------------------
On 2009-09-06T17:09:42+00:00 Gk-gknw wrote:
caveat to the patch I've posted: the pragma
#pragma GCC diagnostic warning "-Wstrict-prototypes"
also overwrites -Werror - that means if -Werror was specified from commandline then after including prlink.h the above pragma would turn it back into warnings.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/22
------------------------------------------------------------------------
On 2009-09-12T19:02:35+00:00 Wan-Teh Chang wrote:
(From update of attachment 398986)
Guenter,
Thanks a lot for this patch. Since prlink.h is a header
file included by many non-NSPR files, it'd be inappropriate
for us (NSPR) to turn on the -Wstrict-prototypes warning as
a result of including prlink.h. So I'm sorry that I can't
accept this patch.
It's too bad that "#pragma GCC diagnostic" doesn't have
an option for restoring to the original setting of a
warning.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/23
------------------------------------------------------------------------
On 2009-09-12T19:10:26+00:00 Wan-Teh Chang wrote:
(In reply to comment #12)
Guenter, could you attach your patch for cert.h to NSS
bug 515870 and ask me to review it? Thanks.
For this bug, I'll just check in rlaager@xxxxxxxxxx's
patch. That patch requires testing with several compilers
we need to support -- we need to make sure by silencing
this GCC warning, we don't break compilation or introduce
warnings with other compilers. So it's not as trivial
as it seems.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/24
------------------------------------------------------------------------
On 2009-09-13T03:39:53+00:00 Gk-gknw wrote:
Hi Wan-Teh Chang,
(In reply to comment #17)
> Guenter, could you attach your patch for cert.h to NSS
> bug 515870 and ask me to review it? Thanks.
I guess thats not needed - after I posted to this bug I checked here:
http://mxr.mozilla.org/security/source/security/nss/lib/certdb/cert.h
and from that it seems that my cert.h patch went already in before I even posted it :)
I came over these warnings with 3.12.3, but surprisingly with 3.12.4 they were gone, so I checked MXR ...
> For this bug, I'll just check in rlaager@xxxxxxxxxx's
> patch. That patch requires testing with several compilers
> we need to support -- we need to make sure by silencing
> this GCC warning, we don't break compilation or introduce
> warnings with other compilers. So it's not as trivial
> as it seems.
Yes, the void patch seems to be the best approach to try. But you should probably attach a test sample here which others can pick up, and check on their platform if it doesnt harm.
thanks, Günter.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/25
------------------------------------------------------------------------
On 2009-09-14T10:14:04+00:00 Wan-Teh Chang wrote:
You're right. Someone else submitted the same patch for cert.h
in bug 491919.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/26
------------------------------------------------------------------------
On 2010-04-26T16:04:08+00:00 Rrelyea wrote:
> Since prlink.h is a public header file and this changes two typedefs, it
> may break source compatibility, so I'm afraid that we can't take this patch.
Sigh, the world is complicated. We just had an instance where this broke
source code compatibility in NSS because prlink.h was added to a new NSS
public header and the lack of ansi compatibility broke several packages.
I just did a search through the NSPR headers and found numerous uses of
void in void returning functions, including such mainstays as
PR_NewLock(). Is there really any doubt that all compiliers that handle
NSPR will handle the voids just fine?
bob
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/27
------------------------------------------------------------------------
On 2010-04-26T16:22:39+00:00 Wan-Teh Chang wrote:
Bob, shall we check in rlaager@xxxxxxxxxx's patch (attachment 295261)
as I proposed in comment 17? Will that fix the problem you just reported?
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/28
------------------------------------------------------------------------
On 2010-06-29T15:36:04+00:00 Rrelyea wrote:
>as I proposed in comment 17? Will that fix the problem you just
reported?
yes, and yes. Elio, no vuvuzela's please.
Reply at: https://bugs.launchpad.net/nspr/+bug/180179/comments/29
--
[PATCH] Fix nspr warnings with -Wstrict-prototypes
https://bugs.launchpad.net/bugs/180179
You received this bug notification because you are a member of Registry
Administrators, which is the registrant for Fedora.
