459481 - NSS build problem with GCC 3.4.6 on OS/2

Status: RESOLVED FIXED

(NSS :: Build, defect, P2)

Description

[Peter Weilbacher]

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]

(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]

(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]

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)]

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]

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]

Attachment: 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]

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]

Attachment: don't compile ffs on OS/2 since it's now part of the gcc headers and runtime library

Comment 9

[Julien Pierre]

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)]

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)]

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]

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]

Thanks for the help, Julien!