Last Comment Bug 459481 - NSS build problem with GCC 3.4.6 on OS/2
: NSS build problem with GCC 3.4.6 on OS/2
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Build (show other bugs)
: 3.12
: x86 OS/2
: P2 normal (vote)
: 3.12.2
Assigned To: Julien Pierre
:
Mentors:
Depends on:
Blocks: 454956
  Show dependency treegraph
 
Reported: 2008-10-11 01:02 PDT by Peter Weilbacher
Modified: 2008-10-15 23:14 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
simple fix (404 bytes, patch)
2008-10-12 14:46 PDT, Peter Weilbacher
julien.pierre: superreview-
Details | Diff | Review
don't compile ffs on OS/2 since it's now part of the gcc headers and runtime library (1000 bytes, patch)
2008-10-13 18:09 PDT, Julien Pierre
nelson: review+
Details | Diff | Review

Description Peter Weilbacher 2008-10-11 01:02:29 PDT
As I discovered in bug 454956 comment 7 there is a weird build problem on OS/2 when using the newly created GCC 3.4.6. It complains (with an error!) about the ffs (re)definition in security/nss/cmd/lib/secutil.h (it is already defined as int ffs(int) in string.h). So we have to somehow exclude it, perhaps by changing
   #ifndef XP_UNIX
to
   #if !defined(XP_UNIX) && !(defined(XP_OS2) &&
       (__GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 4)))
but I am not sure yet if the problem actually also occurs with GCC 3.3.5 or with GCC >= 4.
Comment 1 Walter Meinl 2008-10-11 01:10:49 PDT
(In reply to comment #0)
> As I discovered in bug 454956 comment 7 there is a weird build problem on OS/2
> when using the newly created GCC 3.4.6. It complains (with an error!) about the
> ffs (re)definition in security/nss/cmd/lib/secutil.h (it is already defined as
> int ffs(int) in string.h). So we have to somehow exclude it, perhaps by
> changing
>    #ifndef XP_UNIX
> to
>    #if !defined(XP_UNIX) && !(defined(XP_OS2) &&
>        (__GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 4)))
> but I am not sure yet if the problem actually also occurs with GCC 3.3.5 or
> with GCC >= 4.
    #if !defined(XP_UNIX) && !(defined(XP_OS2) &&
        (__GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 4 || __GNUC__ >= 4)))
GCC-4.3.2 fails as well, GCC-3.3.5 not yet tested, but the file secutil.h is identical in the mercurial and cvs repositories, therefore I assume the ffs definition is probably needed for GCC-3.3.5
Comment 2 Walter Meinl 2008-10-11 01:19:41 PDT
(In reply to comment #1)
Hm, should take myself more time to read more carefully, you already included __GNUC__ > 3, but maybe we have to raise it to >= 4, if we need it for gcc-3.3.5?

>     #if !defined(XP_UNIX) && !(defined(XP_OS2) &&
>         (__GNUC__ >=4 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 4)))
Comment 3 Walter Meinl 2008-10-11 06:56:23 PDT
I googled around and ffs is a built-in function of gcc. I therefore tried with
#if !defined(XP_UNIX) && !defined(XP_OS2)
to build with gcc-3.3.5, posting from this build. Probably it's not needed for any GCC build.
Comment 4 Nelson Bolyard (seldom reads bugmail) 2008-10-11 14:53:38 PDT
Or maybe we do, as we have done in the past, and declare that certain 
versions of GCC are broken and we do not support them.
Comment 5 Peter Weilbacher 2008-10-12 07:46:41 PDT
Well, if you want to de-support GCC 3.3.x and earlier I guess that would be OK, as at least for XPCOM it's the same. Or did you meant to say that 3.4.x and later are broken?
Comment 6 Peter Weilbacher 2008-10-12 14:46:21 PDT
Created attachment 342813 [details] [diff] [review]
simple fix

CVS blame doesn't go back far enough to see why XP_UNIX was originally excluded here, but this seems to be the most simple possible fix. 

This is supposed to go into the version of NSS that is likely to be taken next on mozilla-central (not sure what version to set for this).
Comment 7 Julien Pierre 2008-10-13 17:56:44 PDT
Comment on attachment 342813 [details] [diff] [review]
simple fix

Who is calling this ffs function ? I didn't see any callers. If there are none, let's just delete the code.

And if there are some, shouldn't the ifdef be the same in ffs.c and secutil.h ?
Comment 8 Julien Pierre 2008-10-13 18:09:41 PDT
Created attachment 342977 [details] [diff] [review]
don't compile ffs on OS/2 since it's now part of the gcc headers and runtime library
Comment 9 Julien Pierre 2008-10-13 18:10:49 PDT
I found that there is a single caller for ffs() which is the checkcert program, so the code had to stay . The only remaining special case really is Windows now, since its runtime library does not have a header or an implementation for ffs.
Comment 10 Nelson Bolyard (seldom reads bugmail) 2008-10-15 13:46:19 PDT
Comment on attachment 342977 [details] [diff] [review]
don't compile ffs on OS/2 since it's now part of the gcc headers and runtime library

I'm guessing this is a patch.
Comment 11 Nelson Bolyard (seldom reads bugmail) 2008-10-15 13:47:25 PDT
Comment on attachment 342977 [details] [diff] [review]
don't compile ffs on OS/2 since it's now part of the gcc headers and runtime library

r=nelson
Comment 12 Julien Pierre 2008-10-15 16:36:01 PDT
Thanks for the review, Nelson. I checked this in to the trunk of NSS.

Checking in lib/ffs.c;
/cvsroot/mozilla/security/nss/cmd/lib/ffs.c,v  <--  ffs.c
new revision: 1.3; previous revision: 1.2
done
Checking in lib/secutil.h;
/cvsroot/mozilla/security/nss/cmd/lib/secutil.h,v  <--  secutil.h
new revision: 1.30; previous revision: 1.29
done
Comment 13 Peter Weilbacher 2008-10-15 23:14:19 PDT
Thanks for the help, Julien!

Note You need to log in before you can comment on or make changes to this bug.