Closed
Bug 261649
Opened 20 years ago
Closed 19 years ago
[PATCH] GNU/k*BSD support
Categories
(NSPR :: NSPR, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
4.7
People
(Reporter: rmh, Assigned: wtc)
Details
Attachments
(3 files, 9 obsolete files)
14.03 KB,
patch
|
Details | Diff | Splinter Review | |
760 bytes,
patch
|
Details | Diff | Splinter Review | |
1.12 KB,
patch
|
Biesinger
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040823 Firefox/0.9.3 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040823 Firefox/0.9.3 This patch fixes NSPR to build and pass the testsuite on GNU/k*BSD. It also includes preliminar fixes to support GNU/Hurd. Reproducible: Always Steps to Reproduce: 1. 2. 3.
Reporter | ||
Comment 1•20 years ago
|
||
Assignee | ||
Comment 2•20 years ago
|
||
Thanks for the patch. I will review it.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Reporter | ||
Comment 3•20 years ago
|
||
I noticed a pair of errors in my previous patch (the findstring checks were not correct). I'm adding an update. Will you review this soon?
Assignee | ||
Comment 4•20 years ago
|
||
Robert, Could you tell me a little about GNU/k*BSD and its relation to Hurd, Linux, and FreeBSD? Your patch seems to identify GNU/k*BSD by the "GLIBC" macro. So I suspect GNU/k*BSD is glibc on top of FreeBSD kernel? I am also wondering why you don't just use the __GLIBC__ macro.
Reporter | ||
Comment 5•19 years ago
|
||
Excuse me for the delay. I've been busy working on the other parts of mozilla. (In reply to comment #4) > Robert, > > Could you tell me a little about GNU/k*BSD > and its relation to Hurd, Linux, and FreeBSD? Sure! GNU/kFreeBSD is a system with GNU userland and libc on kernel of FreeBSD. You can learn more about it in http://www.debian.org/ports/kfreebsd-gnu My patch puts it together with other systems that are known to use Glibc. The exception to that is Linux, for which we only know the kernel and can't assume Glibc. > Your patch seems to identify GNU/k*BSD by > the "GLIBC" macro. So I suspect GNU/k*BSD > is glibc on top of FreeBSD kernel? Yes, that is the essential. > I am also wondering why you don't just use > the __GLIBC__ macro. The problem with __GLIBC__ is you can't assume it's defined for all Glibc systems in all situations. Usualy __GLIBC__ is defined in features.h, so if that header hasn't been included (either directly or indirectly) the macro is not defined. On GNU/kFreeBSD, however, __GLIBC__ is a preprocessor macro. The following check defines GLIBC reliing on preprocessor macros exclussively: +#if !defined(GLIBC) && (defined(__GNU__) || defined(__GLIBC__)) +#define GLIBC +#endif
Reporter | ||
Comment 6•19 years ago
|
||
ping
Updated•19 years ago
|
Attachment #171766 -
Flags: review?(wtchang)
Assignee | ||
Comment 7•19 years ago
|
||
Robert, you wrote: My patch puts [GNU/kFreeBSD] together with other systems that are known to use Glibc. The exception to that is Linux, for which we only know the kernel and can't assume Glibc. I don't understand why we can't assume Linux uses Glibc. If we can assume Linux uses Glibc, all the instances of defined(LINUX) || defined(GLIBC) in your patch can be replaced by defined(GLIBC) Is "GLIBC" the standard feature macro to indicate a Glibc system? If every Glibc system has <features.h>, can we just include <features.h> so we can test the __GLIBC__ macro?
Assignee | ||
Comment 8•19 years ago
|
||
Comment on attachment 171766 [details] [diff] [review] new patch >+ case "$target_os" in >+ linux*) AC_DEFINE(LINUX) ;; >+ gnu* | k*bsd*-gnu) AC_DEFINE(GLIBC) ;; >+ esac How do we determine if a Linux system uses Glibc? >diff -ur nsprpub.old/pr/include/md/_linux.cfg nsprpub/pr/include/md/_linux.cfg >--- nsprpub.old/pr/include/md/_linux.cfg 2004-09-24 20:29:02.000000000 +0200 >+++ nsprpub/pr/include/md/_linux.cfg 2004-09-25 13:43:44.000000000 +0200 >@@ -42,10 +42,14 @@ ... >-#ifndef LINUX >+#if !defined(LINUX) && defined(__linux__) > #define LINUX > #endif This definitely needs a comment to explain that _linux.cfg is now used by not only Linux but also other Glibc systems. >+#if !defined(GLIBC) && (defined(__GNU__) || defined(__GLIBC__)) >+#define GLIBC >+#endif This should removed. _linux.cfg is a public header file used by not only NSPR itself but also NSPR clients. I don't want to define GLIBC in a public header if I can avoid it. When compiling NSPR, we already have -DGLIBC on the compiler command line, so NSPR itself doesn't need this. Does __GNU__ mean GNU/Hurd? >diff -ur nsprpub.old/pr/include/md/_pth.h nsprpub/pr/include/md/_pth.h >--- nsprpub.old/pr/include/md/_pth.h 2004-09-24 20:29:02.000000000 +0200 >+++ nsprpub/pr/include/md/_pth.h 2004-09-25 17:44:15.000000000 +0200 >@@ -139,10 +139,14 @@ ... > #elif defined(IRIX) || defined(OSF1) || defined(AIX) || defined(SOLARIS) \ >- || defined(HPUX) || defined(LINUX) || defined(FREEBSD) \ >+ || defined(HPUX) || defined(LINUX) || defined(GLIBC) || defined(FREEBSD) \ ...) >+#ifdef __GNU__ >+# error Using Hurd pthreads? >+/* Hurd pthreads don't have an invalid value for pthread_t. -- rmh */ >+#endif I am a little worried about this #error statement. >diff -ur nsprpub.old/pr/src/md/unix/unix.c nsprpub/pr/src/md/unix/unix.c >--- nsprpub.old/pr/src/md/unix/unix.c 2004-09-24 20:29:02.000000000 +0200 >+++ nsprpub/pr/src/md/unix/unix.c 2004-09-24 20:24:02.000000000 +0200 >@@ -65,7 +65,7 @@ > * PRInt32* pointer to a _PRSockLen_t* pointer. > */ > #if defined(HAVE_SOCKLEN_T) \ >- || (defined(LINUX) && defined(__GLIBC__) && __GLIBC__ >= 2) >+ || (defined(__GLIBC__) && __GLIBC__ >= 2) Why can we omit defined(LINUX) here and not replace it by defined(GLIBC)? >diff -ur nsprpub.old/pr/src/md/unix/uxrng.c nsprpub/pr/src/md/unix/uxrng.c >--- nsprpub.old/pr/src/md/unix/uxrng.c 2004-09-24 20:29:02.000000000 +0200 >+++ nsprpub/pr/src/md/unix/uxrng.c 2004-09-25 17:34:05.000000000 +0200 >@@ -138,7 +138,7 @@ ... >-#elif (defined(LINUX) || defined(FREEBSD) || defined(NETBSD) || defined(OPENBSD)) >+#elif (defined(LINUX) || defined(__FreeBSD_kernel__) || defined(__NetBSD_kernel__) || defined(OPENBSD)) In other places, you handle this by adding defined(GLIBC). This avoids the need to define __FreeBSD_kernel__ and __NetBSD_kernel__ on FreeBSD and NetBSD. Why the difference? >diff -ur nsprpub.old/pr/tests/Makefile.in nsprpub/pr/tests/Makefile.in >--- nsprpub.old/pr/tests/Makefile.in 2004-09-24 20:29:02.000000000 +0200 >+++ nsprpub/pr/tests/Makefile.in 2004-09-24 20:24:02.000000000 +0200 >@@ -376,8 +376,8 @@ > EXTRA_LIBS = -lsocket -lnsl -lgen -lresolv > endif > >-ifeq ($(OS_ARCH), Linux) >- ifeq ($(OS_RELEASE), 1.2) >+ifneq (,$(filter $(OS_ARCH),Linux GNU GNU_kFreeBSD GNU_kNetBSD)) >+ ifeq ($(OS_ARCH)-$(OS_RELEASE), Linux-1.2) > EXTRA_LIBS = -ldl > else > LDOPTS += -Xlinker -rpath $(ABSOLUTE_LIB_DIR) Does GNU mean GNU/Hurd?
Reporter | ||
Comment 9•19 years ago
|
||
> I don't understand why we can't assume Linux uses Glibc. Well I don't really mind. If you prefer to assume it, then we do. > If we can assume Linux uses Glibc, all the instances of > defined(LINUX) || defined(GLIBC) > in your patch can be replaced by > defined(GLIBC) Ok. > Is "GLIBC" the standard feature macro to indicate a Glibc > system? No, __GLIBC__ is the standard. > If every Glibc system has <features.h>, can we just include > <features.h> so we can test the __GLIBC__ macro? This is a chicken-and-egg problem. This header is Glibc-specific so including it to check wether we're on Glibc will cause error for non-Glibc systems before we can even check. This is why, on GNU/kFreeBSD, __GLIBC__ is a C preprocessor macro and can be checked at any time. So if you want a generic check for Glibc, and assuming Linux uses Glibc: #if defined(__linux__) || defined(__GNU__) || defined(__GLIBC__) /* this catches all Glibc-based systems that exist currently, and probably future Glibc-based systems if they're wise and define __GLIBC__ too */ #endif A sub-optimal alternative is including a standard system header like stdio.h, which in turn includes features.h when we're using Glibc. > (From update of attachment 171766 [details] [diff] [review] [edit]) > >+ case "$target_os" in > >+ linux*) AC_DEFINE(LINUX) ;; > >+ gnu* | k*bsd*-gnu) AC_DEFINE(GLIBC) ;; > >+ esac > > How do we determine if a Linux system uses Glibc? Earlier you said we'd rather assume it? It can be checked in many ways but all boil down to the __GLIBC__ macro in features.h. > >diff -ur nsprpub.old/pr/include/md/_linux.cfg nsprpub/pr/include/md/_linux.cfg > >--- nsprpub.old/pr/include/md/_linux.cfg 2004-09-24 20:29:02.000000000 +0200 > >+++ nsprpub/pr/include/md/_linux.cfg 2004-09-25 13:43:44.000000000 +0200 > >@@ -42,10 +42,14 @@ > ... > >-#ifndef LINUX > >+#if !defined(LINUX) && defined(__linux__) > > #define LINUX > > #endif > > This definitely needs a comment to explain that _linux.cfg > is now used by not only Linux but also other Glibc systems. Ok. > >+#if !defined(GLIBC) && (defined(__GNU__) || defined(__GLIBC__)) > >+#define GLIBC > >+#endif > > This should removed. _linux.cfg is a public header file used > by not only NSPR itself but also NSPR clients. I don't want to > define GLIBC in a public header if I can avoid it. When compiling > NSPR, we already have -DGLIBC on the compiler command line, so > NSPR itself doesn't need this. It's been long since I wrote this, but I recall this redundancy was added because -DGLIBC didn't apply to this file in some situations. I'll check again later. > Does __GNU__ mean GNU/Hurd? Yes. > > #elif defined(IRIX) || defined(OSF1) || defined(AIX) || defined(SOLARIS) \ > >- || defined(HPUX) || defined(LINUX) || defined(FREEBSD) \ > >+ || defined(HPUX) || defined(LINUX) || defined(GLIBC) || defined(FREEBSD) \ > ...) > >+#ifdef __GNU__ > >+# error Using Hurd pthreads? > >+/* Hurd pthreads don't have an invalid value for pthread_t. -- rmh */ > >+#endif > > I am a little worried about this #error statement. Would you feel better with a #warning? My intention was merely for the problem not to remain unnoticed when this library is ported to GNU/Hurd. > >diff -ur nsprpub.old/pr/src/md/unix/unix.c nsprpub/pr/src/md/unix/unix.c > >--- nsprpub.old/pr/src/md/unix/unix.c 2004-09-24 20:29:02.000000000 +0200 > >+++ nsprpub/pr/src/md/unix/unix.c 2004-09-24 20:24:02.000000000 +0200 > >@@ -65,7 +65,7 @@ > > * PRInt32* pointer to a _PRSockLen_t* pointer. > > */ > > #if defined(HAVE_SOCKLEN_T) \ > >- || (defined(LINUX) && defined(__GLIBC__) && __GLIBC__ >= 2) > >+ || (defined(__GLIBC__) && __GLIBC__ >= 2) > > Why can we omit defined(LINUX) here and not replace it > by defined(GLIBC)? It doesn't really matter. In this context either GLIBC or __GLIBC__ imply we're using Glibc which is what the "__GLIBC__ >= 2" test is about. > >diff -ur nsprpub.old/pr/src/md/unix/uxrng.c nsprpub/pr/src/md/unix/uxrng.c > >--- nsprpub.old/pr/src/md/unix/uxrng.c 2004-09-24 20:29:02.000000000 +0200 > >+++ nsprpub/pr/src/md/unix/uxrng.c 2004-09-25 17:34:05.000000000 +0200 > >@@ -138,7 +138,7 @@ > ... > >-#elif (defined(LINUX) || defined(FREEBSD) || defined(NETBSD) || defined(OPENBSD)) > >+#elif (defined(LINUX) || defined(__FreeBSD_kernel__) || defined(__NetBSD_kernel__) || defined(OPENBSD)) > > In other places, you handle this by adding defined(GLIBC). > This avoids the need to define __FreeBSD_kernel__ and > __NetBSD_kernel__ on FreeBSD and NetBSD. > > Why the difference? Because the code that follows this check is kernel-specific but not libc-specific. We can't assume it'll work for any Glibc system. I don't remember what the code in question was. Maybe something related to errno macro values (which are defined by kernel) or /proc filesystem? > >-ifeq ($(OS_ARCH), Linux) > >- ifeq ($(OS_RELEASE), 1.2) > >+ifneq (,$(filter $(OS_ARCH),Linux GNU GNU_kFreeBSD GNU_kNetBSD)) > >+ ifeq ($(OS_ARCH)-$(OS_RELEASE), Linux-1.2) > > EXTRA_LIBS = -ldl > > else > > LDOPTS += -Xlinker -rpath $(ABSOLUTE_LIB_DIR) > > Does GNU mean GNU/Hurd? Yes. You're exploring quite far the different possibilities of improving this patch. Not that I don't like it, but I'm a bit confused at what you want. Basicaly if you want me to supply a new patch I'd need to know: - Can we assume Linux uses Glibc? - If so, are you satisfied by a "defined(__linux__) || defined(__GNU__) || defined(__GLIBC__)" check? - Otherwise, which of these methods do you prefer for detecting Glibc on Linux?: - autoconf check for features.h followed by __GLIBC__ check in code. - include a dummy header (e.g. stdio.h) then check for __GLIBC__ - check for $host_os values and define GLIBC on linux*|k*bsd*-gnu|gnu*
Assignee | ||
Comment 10•19 years ago
|
||
It'll probably be easier if I ask you to test my edited version of your patch. Here it is. I will next describe what changes in your patch I omitted and why.
Assignee | ||
Comment 11•19 years ago
|
||
Comment on attachment 171766 [details] [diff] [review] new patch Here is a summary of the changes in your patch that I omitted. For this bug please use the tip of the NSPR source tree (cvs -q co -A mozilla/nsprpub). In nsprpub/config/nsinstall.c, you have: >-#ifdef LINUX >+#if defined(LINUX) || defined(GLIBC) > #include <getopt.h> > #endif The standard header that declares getopt(3) is <unistd.h>. In older versions of glibc, getopt(3) is only declared in <getopt.h>. I believe that with the versions of glibc you are using you can just include <unistd.h>. In nsprpub/configure.in, I don't define __FreeBSD_kernel__ and __NetBSD_kernel__. This means in the C files I need to say defined(FREEBSD) || defined(__FreeBSD_kernel__) defined(NETBSD) || defined(__NetBSD_kernel__) Similarly, I don't define GLIBC, so in the C files I need to say defined(LINUX) || defined(__GNU__) || defined(__GLIBC__) In nsprpub/pr/include/md/_linux.cfg, I don't define GLIBC. In nsprpub/pr/src/md/prosdep.c, I use defined(__GNU__) || defined(__GLIBC__) because your patch uses defined(GLIBC). Since getpagesize() is a system call, it may be more correct to use defined(__FreeBSD_kernel__) || defined(__NetBSD_kernel__) instead. In this case it doesn't matter which one we use because Linux, FreeBSD, and NetBSD all use getpagesize(). In nsprpub/pr/src/md/unix/uxrng.c, I now understand why you use defined(__FreeBSD_kernel__) || defined(__NetBSD_kernel__); the code in question tries to open /dev/random, which is a kernel feature. But in this case it doesn't matter which macros we use either because Linux, FreeBSD, and NetBSD all use the same code. In nsprpub/pr/src/pthreads/ptio.c, I omitted your change related to sendfile for two reasons. 1. sendfile is a system call, so it's probably more correct to use defined(__FreeBSD_kernel__) || defined(__NetBSD_kernel__) than defined(GLIBC). 2. We'd need to make similar changes to pt_linux_sendfile_cont and pt_LinuxSendFile. I omitted your change related to PRSockOptVal_t because that code has been removed on the NSPR tip. In nsprpub/pr/tests/Makefile.in, I omitted your change related to ECHO because the "runtests" makefile target is obsolete. You should instead use the shell script nsprpub/pr/tests/runtests.sh to run our test suite.
Attachment #171766 -
Flags: review?(wtchang)
Assignee | ||
Comment 12•19 years ago
|
||
Comment on attachment 171766 [details] [diff] [review] new patch Another change I made to nsprpub/pr/tests/Makefile.in is to drop support for Linux 1.2 kernel.
Reporter | ||
Comment 13•19 years ago
|
||
(In reply to comment #11) > (From update of attachment 171766 [details] [diff] [review] [edit]) > Here is a summary of the changes in your patch that I omitted. Your patch didn't apply cleanly. I'm attaching an update of the patch you just added with the minor changes to apply to HEAD. In addition, I'm attaching a fix for the getopt problem (see below). After appliing both of these, we build and pass testsuite succesfuly. > In nsprpub/config/nsinstall.c, you have: > > >-#ifdef LINUX > >+#if defined(LINUX) || defined(GLIBC) > > #include <getopt.h> > > #endif > > The standard header that declares getopt(3) is <unistd.h>. In > older versions of glibc, getopt(3) is only declared in <getopt.h>. > I believe that with the versions of glibc you are using you can > just include <unistd.h>. This is correct, except <unistd.h> only declares getopt(3) when _GNU_SOURCE has been enabled. My attached patch does just that. > In nsprpub/pr/src/md/prosdep.c, I use defined(__GNU__) || defined(__GLIBC__) > because your patch uses defined(GLIBC). Since getpagesize() is a system > call, it may be more correct to use defined(__FreeBSD_kernel__) || > defined(__NetBSD_kernel__) instead. In this case it doesn't matter > which one we use because Linux, FreeBSD, and NetBSD all use getpagesize(). But maybe GNU/Hurd doesn't. I think it should be defined(__FreeBSD_kernel__) || defined(__NetBSD_kernel__) as you say. > In nsprpub/pr/src/pthreads/ptio.c, I omitted your change related to > sendfile for two reasons. 1. sendfile is a system call, so it's probably > more correct to use defined(__FreeBSD_kernel__) || defined(__NetBSD_kernel__) > than defined(GLIBC). Actualy we don't have this syscall (it's disabled in glibc to avoid trouble), so I assume no change is needed.
Reporter | ||
Comment 14•19 years ago
|
||
Reporter | ||
Updated•19 years ago
|
Attachment #160149 -
Attachment is obsolete: true
Attachment #171766 -
Attachment is obsolete: true
Reporter | ||
Comment 15•19 years ago
|
||
Reporter | ||
Comment 16•19 years ago
|
||
I forgot to mention that config.guess and config.sub need to be updated to a more recent version in order to detect GNU/kFreeBSD. Thanks!
Assignee | ||
Comment 17•19 years ago
|
||
Comment on attachment 188688 [details] [diff] [review] getopt fix In nsprpub/pr/src/linking/Makefile.in, you have: > # For Dl_info and dladdr. >-ifeq ($(OS_TARGET),Linux) >+ifeq (,$(filter-out Linux GNU GNU_kFreeBSD GNU_kNetBSD,$(OS_TARGET))) > DEFINES += -D_GNU_SOURCE > endif This change is useless without a corresponding change to nsprpub/pr/src/linking/prlink.c (around dladdr).
Reporter | ||
Comment 18•19 years ago
|
||
Ok. Wrt dladdr thing, a __GLIBC__ check will do it. diff -x configure -ur nsprpub.old/pr/src/linking/prlink.c nsprpub/pr/src/linking/prlink.c --- nsprpub.old/pr/src/linking/prlink.c 2005-07-02 00:26:36.000000000 +0200 +++ nsprpub/pr/src/linking/prlink.c 2005-07-09 13:46:24.000000000 +0200 @@ -1689,7 +1689,7 @@ PR_IMPLEMENT(char *) PR_GetLibraryFilePathname(const char *name, PRFuncPtr addr) { -#if defined(SOLARIS) || defined(LINUX) || defined(FREEBSD) +#if defined(SOLARIS) || defined(LINUX) || defined(FREEBSD) || defined(__GLIBC__) Dl_info dli; char *result;
Reporter | ||
Comment 19•19 years ago
|
||
complete patch with wtchang's approved parts plus _GNU_SOURCE definitions (in configure.in and pr/src/linking/Makefile.in) for getopt and dladdr. diffed against current HEAD. Please remember to update config.{guess,sub}!
Attachment #188687 -
Attachment is obsolete: true
Attachment #188688 -
Attachment is obsolete: true
Reporter | ||
Comment 20•19 years ago
|
||
Hi! This bug is almost a year old. I think everything is sorted out now, is the patch ready for commit?
Reporter | ||
Comment 21•19 years ago
|
||
Hi! I'm very interested in this bug. Is there something I can do to get the patch applied? It seems to have been stalled.
Updated•19 years ago
|
Attachment #188966 -
Flags: review?(wtchang)
Reporter | ||
Comment 22•19 years ago
|
||
Ping..
Assignee | ||
Comment 23•19 years ago
|
||
Thanks a lot for the new patch. It looks good. I only made some minor changes, most of which are formatting changes. 1. mozilla/nsprpub/configure.in I removed AC_DEFINE(_GNU_SOURCE). mozilla/nsprpub/pr/src/linking/prlink.c is the only file that needs to be compiled with -D_GNU_SOURCE, and we take care of that in mozilla/nsprpub/pr/src/linking/Makefile.in. 2. mozilla/nsprpub/pr/include/md/_pth.h I'm wondering why you didn't add __GNU__ and __GLIBC__ to the test for PT_NO_SIGTIMEDWAIT. Is it because sigtimedwait is a kernel feature (system call)? 3. mozilla/nsprpub/pr/src/linking/prlink.c Why didn't you add defined(__GNU__) ? 4. In the Makefile.in's, you can use GNU_% in place of GNU_kFreeBSD GNU_kNetBSD Our makefiles change / to _ in the uname -s output. Your PORTING guide suggests we use GNU/* in a uname check. The equivalent in our makefiles is GNU_%. (% is GNU Make's wildcard in pattern matching.) I've checked in this patch on the NSPR trunk (NSPR 4.7). I will check it in on the NSPRPUB_PRE_4_2_CLIENT_BRANCH, which is used by the Mozilla trunk, tomorrow. Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 1.208; previous revision: 1.207 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 1.210; previous revision: 1.209 done Checking in pr/include/md/_linux.cfg; /cvsroot/mozilla/nsprpub/pr/include/md/_linux.cfg,v <-- _linux.cfg new revision: 3.19; previous revision: 3.18 done Checking in pr/include/md/_pth.h; /cvsroot/mozilla/nsprpub/pr/include/md/_pth.h,v <-- _pth.h new revision: 3.31; previous revision: 3.30 done Checking in pr/include/md/_unixos.h; /cvsroot/mozilla/nsprpub/pr/include/md/_unixos.h,v <-- _unixos.h new revision: 3.36; previous revision: 3.35 done Checking in pr/include/md/prosdep.h; /cvsroot/mozilla/nsprpub/pr/include/md/prosdep.h,v <-- prosdep.h new revision: 3.17; previous revision: 3.16 done Checking in pr/src/linking/Makefile.in; /cvsroot/mozilla/nsprpub/pr/src/linking/Makefile.in,v <-- Makefile.in new revision: 1.16; previous revision: 1.15 done Checking in pr/src/linking/prlink.c; /cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v <-- prlink.c new revision: 3.83; previous revision: 3.82 done Checking in pr/src/md/prosdep.c; /cvsroot/mozilla/nsprpub/pr/src/md/prosdep.c,v <-- prosdep.c new revision: 3.12; previous revision: 3.11 done Checking in pr/src/md/unix/unix.c; /cvsroot/mozilla/nsprpub/pr/src/md/unix/unix.c,v <-- unix.c new revision: 3.52; previous revision: 3.51 done Checking in pr/src/md/unix/uxproces.c; /cvsroot/mozilla/nsprpub/pr/src/md/unix/uxproces.c,v <-- uxproces.c new revision: 3.20; previous revision: 3.19 done Checking in pr/src/md/unix/uxrng.c; /cvsroot/mozilla/nsprpub/pr/src/md/unix/uxrng.c,v <-- uxrng.c new revision: 1.19; previous revision: 1.18 done Checking in pr/src/misc/prnetdb.c; /cvsroot/mozilla/nsprpub/pr/src/misc/prnetdb.c,v <-- prnetdb.c new revision: 3.48; previous revision: 3.47 done Checking in pr/src/pthreads/ptio.c; /cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v <-- ptio.c new revision: 3.103; previous revision: 3.102 done Checking in pr/tests/Makefile.in; /cvsroot/mozilla/nsprpub/pr/tests/Makefile.in,v <-- Makefile.in new revision: 1.48; previous revision: 1.47 done
Attachment #188966 -
Attachment is obsolete: true
Attachment #188966 -
Flags: review?(wtchang)
Reporter | ||
Comment 24•19 years ago
|
||
Hi! I just tried current CVS. It needed a minor fix to build (in nsinstall.c), other than this it builds and passes the testsuite. I'm attaching a new patch with that, and the fixes you request below. (after these changes, testsuite also passes ok) (In reply to comment #23) > 2. mozilla/nsprpub/pr/include/md/_pth.h > > I'm wondering why you didn't add __GNU__ and __GLIBC__ > to the test for PT_NO_SIGTIMEDWAIT. Is it because > sigtimedwait is a kernel feature (system call)? My oversight. Yes, I think this can be added. > 3. mozilla/nsprpub/pr/src/linking/prlink.c > > Why didn't you add defined(__GNU__) ? Oversight again. I added it in new patch too. > 4. In the Makefile.in's, you can use > > GNU_% > > in place of > > GNU_kFreeBSD GNU_kNetBSD > > Our makefiles change / to _ in the uname -s output. > Your PORTING guide suggests we use GNU/* in a uname > check. The equivalent in our makefiles is GNU_%. > (% is GNU Make's wildcard in pattern matching.) Ah, I didn't know this feature. I'm adding it too. That's all. See attached new patch. Thanks a lot!
Reporter | ||
Comment 25•19 years ago
|
||
I can't send a new attachment due to Bugzilla internal error, so I'll paste it here instead. Sorry for the inconvenience. Index: nsprpub/config/nsinstall.c =================================================================== RCS file: /cvsroot/mozilla/nsprpub/config/nsinstall.c,v retrieving revision 3.20 diff -u -r3.20 nsinstall.c --- nsprpub/config/nsinstall.c 25 Apr 2004 15:00:34 -0000 3.20 +++ nsprpub/config/nsinstall.c 24 Dec 2005 11:39:07 -0000 @@ -98,7 +98,7 @@ } #endif /* NEXTSTEP */ -#ifdef LINUX +#if defined(LINUX) || defined(__GLIBC__) || defined(__GNU__) #include <getopt.h> #endif Index: nsprpub/pr/include/md/_pth.h =================================================================== RCS file: /cvsroot/mozilla/nsprpub/pr/include/md/_pth.h,v retrieving revision 3.31 diff -u -r3.31 _pth.h --- nsprpub/pr/include/md/_pth.h 24 Dec 2005 08:25:23 -0000 3.31 +++ nsprpub/pr/include/md/_pth.h 24 Dec 2005 11:39:07 -0000 @@ -201,6 +201,7 @@ * These platforms don't have sigtimedwait() */ #if (defined(AIX) && !defined(AIX4_3_PLUS)) || defined(LINUX) \ + || defined(__GLIBC__) || defined(__GNU__) \ || defined(FREEBSD) || defined(NETBSD) || defined(OPENBSD) \ || defined(BSDI) || defined(VMS) || defined(UNIXWARE) \ || defined(DARWIN) Index: nsprpub/pr/src/linking/Makefile.in =================================================================== RCS file: /cvsroot/mozilla/nsprpub/pr/src/linking/Makefile.in,v retrieving revision 1.16 diff -u -r1.16 Makefile.in --- nsprpub/pr/src/linking/Makefile.in 24 Dec 2005 08:25:28 -0000 1.16 +++ nsprpub/pr/src/linking/Makefile.in 24 Dec 2005 11:39:07 -0000 @@ -63,7 +63,7 @@ INCLUDES = -I$(dist_includedir) -I$(topsrcdir)/pr/include -I$(topsrcdir)/pr/include/private # For Dl_info and dladdr. -ifeq (,$(filter-out Linux GNU GNU_kFreeBSD GNU_kNetBSD,$(OS_TARGET))) +ifeq (,$(filter-out Linux GNU GNU_%,$(OS_TARGET))) DEFINES += -D_GNU_SOURCE endif Index: nsprpub/pr/src/linking/prlink.c =================================================================== RCS file: /cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v retrieving revision 3.83 diff -u -r3.83 prlink.c --- nsprpub/pr/src/linking/prlink.c 24 Dec 2005 08:25:28 -0000 3.83 +++ nsprpub/pr/src/linking/prlink.c 24 Dec 2005 11:39:08 -0000 @@ -1389,7 +1389,7 @@ PR_GetLibraryFilePathname(const char *name, PRFuncPtr addr) { #if defined(SOLARIS) || defined(FREEBSD) \ - || defined(LINUX) || defined(__GLIBC__) + || defined(LINUX) || defined(__GLIBC__) || defined(__GNU__) Dl_info dli; char *result; Index: nsprpub/pr/tests/Makefile.in =================================================================== RCS file: /cvsroot/mozilla/nsprpub/pr/tests/Makefile.in,v retrieving revision 1.48 diff -u -r1.48 Makefile.in --- nsprpub/pr/tests/Makefile.in 24 Dec 2005 08:25:31 -0000 1.48 +++ nsprpub/pr/tests/Makefile.in 24 Dec 2005 11:39:08 -0000 @@ -376,7 +376,7 @@ EXTRA_LIBS = -lsocket -lnsl -lgen -lresolv endif -ifeq (,$(filter-out Linux GNU GNU_kFreeBSD GNU_kNetBSD,$(OS_ARCH))) +ifeq (,$(filter-out Linux GNU GNU_%,$(OS_ARCH))) LDOPTS += -Xlinker -rpath $(ABSOLUTE_LIB_DIR) ifeq ($(USE_PTHREADS),1) EXTRA_LIBS = -lpthread
Assignee | ||
Comment 26•19 years ago
|
||
I incorporated the new changes into this patch. I've checked in the new changes on the NSPR trunk (NSPR 4.7) and this patch on the NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla/Gecko trunk/1.9 alpha). Here is the cvs commit output for the NSPRPUB_PRE_4_2_CLIENT_BRANCH: Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 1.78.2.126; previous revision: 1.78.2.125 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 1.83.2.124; previous revision: 1.83.2.123 done Checking in config/nsinstall.c; /cvsroot/mozilla/nsprpub/config/nsinstall.c,v <-- nsinstall.c new revision: 3.17.2.4; previous revision: 3.17.2.3 done Checking in pr/include/md/_linux.cfg; /cvsroot/mozilla/nsprpub/pr/include/md/_linux.cfg,v <-- _linux.cfg new revision: 3.12.4.9; previous revision: 3.12.4.8 done Checking in pr/include/md/_pth.h; /cvsroot/mozilla/nsprpub/pr/include/md/_pth.h,v <-- _pth.h new revision: 3.19.2.14; previous revision: 3.19.2.13 done Checking in pr/include/md/_unixos.h; /cvsroot/mozilla/nsprpub/pr/include/md/_unixos.h,v <-- _unixos.h new revision: 3.31.4.5; previous revision: 3.31.4.4 done Checking in pr/include/md/prosdep.h; /cvsroot/mozilla/nsprpub/pr/include/md/prosdep.h,v <-- prosdep.h new revision: 3.13.4.4; previous revision: 3.13.4.3 done Checking in pr/src/linking/Makefile.in; /cvsroot/mozilla/nsprpub/pr/src/linking/Makefile.in,v <-- Makefile.in new revision: 1.10.2.6; previous revision: 1.10.2.5 done Checking in pr/src/linking/prlink.c; /cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v <-- prlink.c new revision: 3.51.2.29; previous revision: 3.51.2.28 done Checking in pr/src/md/prosdep.c; /cvsroot/mozilla/nsprpub/pr/src/md/prosdep.c,v <-- prosdep.c new revision: 3.9.4.3; previous revision: 3.9.4.2 done Checking in pr/src/md/unix/unix.c; /cvsroot/mozilla/nsprpub/pr/src/md/unix/unix.c,v <-- unix.c new revision: 3.43.2.9; previous revision: 3.43.2.8 done Checking in pr/src/md/unix/uxproces.c; /cvsroot/mozilla/nsprpub/pr/src/md/unix/uxproces.c,v <-- uxproces.c new revision: 3.14.2.6; previous revision: 3.14.2.5 done Checking in pr/src/md/unix/uxrng.c; /cvsroot/mozilla/nsprpub/pr/src/md/unix/uxrng.c,v <-- uxrng.c new revision: 1.11.4.6; previous revision: 1.11.4.5 done Checking in pr/src/misc/prnetdb.c; /cvsroot/mozilla/nsprpub/pr/src/misc/prnetdb.c,v <-- prnetdb.c new revision: 3.21.2.27; previous revision: 3.21.2.26 done Checking in pr/src/pthreads/ptio.c; /cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v <-- ptio.c new revision: 3.71.2.24; previous revision: 3.71.2.23 done Checking in pr/tests/Makefile.in; /cvsroot/mozilla/nsprpub/pr/tests/Makefile.in,v <-- Makefile.in new revision: 1.34.2.14; previous revision: 1.34.2.13 done
Attachment #188364 -
Attachment is obsolete: true
Attachment #206751 -
Attachment is obsolete: true
Reporter | ||
Comment 27•19 years ago
|
||
I just tested your last commit and it builds and passes testsuite. Thank you very much!
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 28•19 years ago
|
||
Attachment #206762 -
Attachment is obsolete: true
Assignee | ||
Comment 29•19 years ago
|
||
I just realized you may have added AC_DEFINE(_GNU_SOURCE) so that you could build mozilla/nsprpub/config/nsinstall.c without the change to include <getopt.h>. I think it may be a good idea to define _GNU_SOURCE (or just remove the -ansi compiler flag), but we don't need to do that in this bug. Now NSPR works on GNU/kFreeBSD and GNU/kNetBSD, but still doesn't work on GNU/Hurd (because of the lack of an invalid value for pthread_t), right? I'm sorry about my long delay in reviewing and checking in your patch. Thanks a lot for your contribution. Merry Christmas! :-)
Whiteboard: [4.7]
Reporter | ||
Comment 30•19 years ago
|
||
> Now NSPR works on GNU/kFreeBSD and GNU/kNetBSD, but still > doesn't work on GNU/Hurd (because of the lack of an invalid > value for pthread_t), right? Yes. You'll have to check with the GNU/Hurd porters to provide another solution -- this is out of my reach. > I'm sorry about my long delay in reviewing and checking in > your patch. Thanks a lot for your contribution. No problem. It was worth the wait! :) Btw, I might propose this to be merged into 1.8 or even 1.8.0 later (along with my other patches for trunk that aren't yet submitted). Do I have to check with you, or just send the proposal? Thank you.
Assignee | ||
Comment 31•19 years ago
|
||
You need to get your patches checked in on the trunk first. The Mozilla 1.8 branch is more likely to accept your patches. The Mozilla 1.8.0.1 branch is mainly for security vulnerability fixes, so the GNU/k*BSD porting patches may not be accepted.
Comment 32•19 years ago
|
||
it looks like this patch may have caused a regression in BeOS. See https://bugzilla.mozilla.org/show_bug.cgi?id=321579
Assignee | ||
Comment 33•19 years ago
|
||
In bug 321579 we learned that BeOS also uses glibc, but either that glibc doesn't have the dlxxx functions declared in <dlfcn.h>, or we (NSPR) choose not to use them. I've asked the BeOS Mozilla developers to review our patch v3 (attachment 206765 [details] [diff] [review]) in this bug. This patch fixes the BeOS build error caused by that patch.
Assignee | ||
Updated•19 years ago
|
Whiteboard: [4.7]
Target Milestone: --- → 4.7
Assignee | ||
Comment 34•19 years ago
|
||
I found that most of the patch (attachment 206765 [details] [diff] [review]) is in Unix-only code. I collected the changes that affect BeOS in this patch. Please review and make sure they don't break BeOS. 1. mozilla/nsprpub/config/nsinstall.c: you just need to verify that BeOS has the <getopt.h> header. 2. mozilla/nsprpub/pr/src/linking/prlink.c: I know this patch works, so I'm just curious as to whether BeOS doesn't have the dlxxx functions (declared in <dlfcn.h>), or we choose not to use them. 3. mozilla/nsprpub/pr/src/misc/prnetdb.c: you need to verify that BeOS has the following two functions (declared in <netdb.h>) and it's ok to use them: extern int getprotobyname_r (__const char *__restrict __name, struct protoent *__restrict __result_buf, char *__restrict __buf, size_t __buflen, struct protoent **__restrict __result); extern int getprotobynumber_r (int __proto, struct protoent *__restrict __result_buf, char *__restrict __buf, size_t __buflen, struct protoent **__restrict __result);
Attachment #207454 -
Flags: superreview?(sergei_d)
Attachment #207454 -
Flags: review?(cbiesinger)
Comment 35•19 years ago
|
||
Comment on attachment 207454 [details] [diff] [review] Subset of patch that affects BeOS hm... I only have Zeta here, which is a pretty new version of beos. so, take this with a grain of salt. I do have getopt.h. I'm pretty sure not all beos systems have libdl, which is why we prefer not to use it, and instead use native beos functions. there's also the issue of the .stub libraries we use... The getproto* functions are declared and can be called, though they seem to just fail... (tested with "tcp" as name, or 6 as number) They have four arguments: - the name/number (const char* or int) - struct protoent* - char* - int (NOTE: I haven't checked whether the non-reentrant versions return meaningful data) So at least the last part here is not ok - there is no 5-arg version of getproto*.
Attachment #207454 -
Flags: review?(cbiesinger) → review-
Assignee | ||
Comment 36•19 years ago
|
||
Christian: do you always define BONE_VERSION when building Mozilla clients on BeOS now? Are getprotobyname_r and getprotobynumber_r declared like the following (ignoring parameter names) on BeOS? struct protoent *getprotobyname_r(const char *name, struct protoent *result, char *buffer, int buflen); struct protoent *getprotobynumber_r(int proto, struct protoent *result, char *buffer, int buflen); Since you already provided the parameters in comment 35, I just need to know if these two functions return struct protoent *.
Comment 37•18 years ago
|
||
sorry, I don't know the answer to the BONE_VERSION question. Yes, the functions do return struct protoent*.
Comment 38•18 years ago
|
||
No, BONE is unfortunatly not supported by plain BeOS R5 which a lot of users depend upon. When building on Zeta BONE should be default, but I've never built from Zeta.
Comment 39•18 years ago
|
||
Wan-Teh, is patch https://bugzilla.mozilla.org/attachment.cgi?id=207454 checked in already? I got strange problem building seamonkey on "classical" BeOS R5 (no BONE), which looks seomewhat related to nsinstall in NSS: http://community.livejournal.com/bezilla/167654.html
Assignee | ||
Comment 40•18 years ago
|
||
Sergei, the NSPR patch (attachment 207454 [details] [diff] [review]) has been checked in on the NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla trunk) and NSPR trunk only. It hasn't been checked in on any other Mozilla branch. Your build errors are in NSS, so are unlikely to be related to this bug. We landed a new NSS (3.11) on the Mozilla trunk in January and on the MOZILLA_1_8_BRANCH last Friday (bug 317620). Even though that's a massive change, it shouldn't cause the strange build problem you are seeing. Let's continue the discussion by email.
Assignee | ||
Comment 41•18 years ago
|
||
As Christian reported in comment 35, BeOS has the getprotobyxxx_r functions, but they aren't glibc's 5-argument version.
Attachment #207454 -
Attachment is obsolete: true
Attachment #213385 -
Flags: review?(cbiesinger)
Attachment #207454 -
Flags: superreview?(sergei_d)
Updated•18 years ago
|
Attachment #213385 -
Flags: review?(cbiesinger) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•