Closed Bug 312361 Opened 19 years ago Closed 19 years ago

Determine if the visibility("default") attribute is supported by using GCC version only

Categories

(NSPR :: NSPR, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wtc, Assigned: wtc)

Details

(Keywords: fixed1.8.1)

Attachments

(1 file)

NSPR header files have always been "self sufficient", which means that our clients don't need to define any macro when including our header files. In bug 273336 we changed that. prtypes.h now tests the HAVE_VISIBILITY_PRAGMA macro, which our callers need to define if the compiler (GCC) supports the visibility pragma: #ifdef HAVE_VISIBILITY_PRAGMA #define PR_VISIBILITY_DEFAULT __attribute__((visibility("default"))) #else #define PR_VISIBILITY_DEFAULT #endif After a lot of research and experiments, I concluded that it is safe to replace that test with a test of the GCC version using GCC's built-in macros.
Attached patch Proposed patchSplinter Review
I looked at the GCC 3.0 - 4.0 manuals and found that the visibility attribute is first documented in the GCC 3.3 manual. Testing the GCC version works provided that no one (individual or Linux distribution) has backported the visibility pragmas or the -fvisibility flag to their special build of GCC 3.2.x or earlier, because the visibility pragmas and the -fvisibility flag are what make it crucial to define PR_VISIBILITY_DEFAULT to be __attribute__((visibility("default"))). -fvisilibility is new in GCC 4.0.0, so I doubt anyone would backport it to GCC 3.2.x or earlier. (Red Hat backported it to its special build of GCC 3.4.x.) I believe that the visibility pragmas were added in the same GCC patch, but I'll need bryner to confirm that. I've tested this patch on - Red Hat Enterprise Linux 3, gcc 3.2 - Red Hat Enterprise Linux 4, gcc 3.4 and gcc 4.0 - Mac OS X (unknown version), gcc 3.3 - Mac OS X Tiger, gcc 4.0
Attachment #199459 - Flags: review?(bryner)
Comment on attachment 199459 [details] [diff] [review] Proposed patch This should be ok, I think.
Attachment #199459 - Flags: review?(bryner) → review+
Patch checked in on the NSPR tip (NSPR 4.7) and the NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.9a).
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: [4.7]
This patch should be carried back to the NSPR_4_6_BRANCH and MOZILLA_1_8_BRANCH.
Whiteboard: [4.7]
Target Milestone: --- → 4.7
Attachment #199459 - Flags: approval1.8.1?
Attachment #199459 - Flags: approval1.8.0.1?
I carried the patch back to the NSPR_4_6_BRANCH (NSPR 4.6.2). Checking in prtypes.h; /cvsroot/mozilla/nsprpub/pr/include/prtypes.h,v <-- prtypes.h new revision: 3.30.2.1; previous revision: 3.30 done
Target Milestone: 4.7 → 4.6.2
Comment on attachment 199459 [details] [diff] [review] Proposed patch not for 1.8.0.1
Attachment #199459 - Flags: approval1.8.1?
Attachment #199459 - Flags: approval1.8.1+
Attachment #199459 - Flags: approval1.8.0.1?
Attachment #199459 - Flags: approval1.8.0.1-
I checked in the patch on the MOZILLA_1_8_BRANCH for Mozilla/Gecko 1.8.1. Checking in prtypes.h; /cvsroot/mozilla/nsprpub/pr/include/prtypes.h,v <-- prtypes.h new revision: 3.20.4.10.22.1; previous revision: 3.20.4.10 done
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: