Closed
Bug 326110
Opened 19 years ago
Closed 19 years ago
PR_SetSysfdTableSize and PR_GetSysfdTableMax are not exported
Categories
(NSPR :: NSPR, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
4.6.2
People
(Reporter: nhosoi, Assigned: wtc)
Details
(Keywords: fixed1.8.1, regression)
Attachments
(2 files)
847 bytes,
patch
|
KaiE
:
review+
julien.pierre
:
superreview+
wtc
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
1.64 KB,
patch
|
KaiE
:
review+
julien.pierre
:
superreview+
wtc
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1 Build Identifier: We are taking Wan-Teh's suggestion and put the ifdef statement to work around the build failure. Wan-Teh wrote: I looked at the NSPR source code and bug database. I believe these two functions are intended to be exported even though they are not documented in the NSPR Reference and not declared in any NSPR header files. In NSPR 4.6, I added a Linux so that only the functions declared in our public headers are exported from our shared libraries. This is why you get undefined references for those two functions on RHEL. If you have an account in bugzilla.mozilla.org, could you file an NSPR bug report? https://bugzilla.mozilla.org/enter_bug.cgi?product=NSPR Meantime, I suggest that you temporarily work around this NSPR bug like this: #ifdef LINUX /* * NSPR 4.6 does not export PR_SetSysfdTableSize * and PR_GetSysfdTableMax by mistake (NSPR Bugzilla * bug xxxxxx) on platforms that use GCC with symbol * visibility, so we have to call the system calls * directly. */ { struct rlimit rlim; if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) return; rlim.rlim_cur = rlim.rlim_max; (void) setrlimit(RLIMIT_NOFILE, &rlim); } #else PR_SetSysfdTableSize(PR_GetSysfdTableMax()); #endif Wan-Teh Reproducible: Always
Assignee | ||
Comment 1•19 years ago
|
||
I considered declaring these two functions in prio.h or prsystem.h. But then I saw this comment before their definitions in ptio.c: /* * The next two entry points should not be in the API, but they are * defined here for historical (or hysterical) reasons. */ So I decided to declare them in "obsolete/probslet.h". Noriko, if you know how to build NSPR, could you apply this patch to NSPR_4_6_RTM and see if it fixes the bug for you?
Attachment #210929 -
Flags: superreview?(julien.pierre.bugs)
Attachment #210929 -
Flags: review?(kengert)
Comment 2•19 years ago
|
||
Comment on attachment 210929 [details] [diff] [review] Proposed patch r=kengert
Attachment #210929 -
Flags: review?(kengert) → review+
Updated•19 years ago
|
Attachment #210929 -
Flags: superreview?(julien.pierre.bugs) → superreview+
Reporter | ||
Comment 3•19 years ago
|
||
(In reply to comment #1) > [...] > /* > * The next two entry points should not be in the API, but they are > * defined here for historical (or hysterical) reasons. > */ > > So I decided to declare them in "obsolete/probslet.h". > > Noriko, if you know how to build NSPR, could you apply this > patch to NSPR_4_6_RTM and see if it fixes the bug for you? > I applied the patch to my NSPR tree (sorry, it's not NSPR_4_6_RTM, but the trunk) and built NSPR. I could build DS fine which pointed to the newly built NSPR. Thank you, Wan-Teh.
Assignee | ||
Comment 4•19 years ago
|
||
I checked in the patch on the NSPR trunk (NSPR 4.7), NSPRPUB_PRE_4_2_CLIENT_BRANCH (mozilla1.9alpha), and NSPR_4_6_BRANCH (NSPR 4.6.2). Checking in probslet.h; /cvsroot/mozilla/nsprpub/pr/include/obsolete/probslet.h,v <-- probslet.h new revision: 3.11; previous revision: 3.10 done Checking in probslet.h; /cvsroot/mozilla/nsprpub/pr/include/obsolete/probslet.h,v <-- probslet.h new revision: 3.8.4.3; previous revision: 3.8.4.2 done Checking in probslet.h; /cvsroot/mozilla/nsprpub/pr/include/obsolete/probslet.h,v <-- probslet.h new revision: 3.9.2.1; previous revision: 3.9 done Note that these two functions are not exported on Windows and were not exported on Windows before. Perhaps we should also use PR_IMPLEMENT with their definitions to cause them to be exported on Windows.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: regression
Priority: -- → P1
Target Milestone: --- → 4.6.2
Version: other → 4.6
Assignee | ||
Comment 5•19 years ago
|
||
I was wrong. Although these two functions were not exported on Windows before, by declaring them with NSPR_API in a public header, we are exporting them on Windows now. It is still a good idea to use PR_IMPLEMENT with these two functions. That's what this patch does. I've checked it on on the NSPR trunk (NSPR 4.7), NSPRPUB_PRE_4_2_CLIENT_BRANCH (mozilla1.9alpha), and NSPR_4_6_BRANCH (NSPR 4.6.2). Checking in io/prfile.c; /cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v <-- prfile.c new revision: 3.44; previous revision: 3.43 done Checking in pthreads/ptio.c; /cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v <-- ptio.c new revision: 3.104; previous revision: 3.103 done Checking in io/prfile.c; /cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v <-- prfile.c new revision: 3.36.2.8; previous revision: 3.36.2.7 done Checking in pthreads/ptio.c; /cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v <-- ptio.c new revision: 3.71.2.25; previous revision: 3.71.2.24 done Checking in io/prfile.c; /cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v <-- prfile.c new revision: 3.42.2.1; previous revision: 3.42 done Checking in pthreads/ptio.c; /cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v <-- ptio.c new revision: 3.101.2.1; previous revision: 3.101 done
Attachment #210952 -
Flags: superreview?(julien.pierre.bugs)
Attachment #210952 -
Flags: review?(kengert)
Assignee | ||
Updated•19 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 6•19 years ago
|
||
Wan-Teh, These 2 functions were already in nspr.def, which I had checked before reviewing the patch. I wasn't aware they weren't supposed to be exported on windows. Is nspr.def not used for all platforms as for NSS ?
Assignee | ||
Comment 7•19 years ago
|
||
Julien, In NSPR, the .def files are currently only used by Solaris and OS/2. We can use the .def files on other platforms. However, we may not be able to use the .def files on Windows because they conflict with the _declspec(dllexport) keyword that PR_IMPLEMENT expands to on Windows. These two functions were not exported on Windows. It's fine to export them, which is why I decided to export them now to have cross-platform consistency.
Updated•19 years ago
|
Attachment #210952 -
Flags: review?(kengert) → review+
Assignee | ||
Comment 8•19 years ago
|
||
I checked in the patches on the MOZILLA_1_8_BRANCH, keeping it in sync with the NSPR_4_6_BRANCH. Checking in pr/include/obsolete/probslet.h; /cvsroot/mozilla/nsprpub/pr/include/obsolete/probslet.h,v <-- probslet.h new revision: 3.8.4.1.32.1; previous revision: 3.8.4.1 done Checking in pr/src/io/prfile.c; /cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v <-- prfile.c new revision: 3.36.2.6.16.1; previous revision: 3.36.2.6 done Checking in pr/src/pthreads/ptio.c; /cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v <-- ptio.c new revision: 3.71.2.21.12.2; previous revision: 3.71.2.21.12.1 done
Keywords: fixed1.8.1
Assignee | ||
Updated•19 years ago
|
Attachment #210929 -
Flags: branch-1.8.1+
Assignee | ||
Updated•19 years ago
|
Attachment #210952 -
Flags: branch-1.8.1+
Updated•18 years ago
|
Attachment #210952 -
Flags: superreview?(julien.pierre.bugs) → superreview+
You need to log in
before you can comment on or make changes to this bug.
Description
•