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 [email protected]'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 [email protected]'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 [email protected]'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. _______________________________________________ Mailing list: https://launchpad.net/~registry Post to : [email protected] Unsubscribe : https://launchpad.net/~registry More help : https://help.launchpad.net/ListHelp
