131306 - NSPR needs to support loading CFM, CFBundle and mach-o libraries

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P1)

Description

[Patrick C. Beard]

Bug #131300 provides support for running CFM browser plugins on Mac OS X. NSPR
also needs to be enhanced to support this. Patches to follow.

Comment 1

[Patrick C. Beard]

Attachment: Patch to prlink.c v1

This enables loading of CFM and CFBundle format plugins.

Comment 2

[Patrick C. Beard]

This needs some more work. There needs to be some kind of autoconf test to
#define XP_MACOSX when prlink.c is compiled. In addition, the Makefile needs to
define

OS_LIBS = -framework Carbon -framework CoreFoundation

not for Darwin, but for Mac OS X builds only.

Comment 3

[Wan-Teh Chang]

Patrick,

As far as NSPR is concerned, what is the difference between
Darwin and Mac OS X?  It seems that NSPR only uses the Darwin
portion of Mac OS X, so I don't understand the distinction
you were trying to make.

Comment 4

[hacker formerly known as [e-mail]]

Wan-Teh, the problem is that Beard's patch uses parts of the Carbon toolkit
which is only available from OSX.  The standalone Darwin distributions will not
have access to that toolkit so we need a way to differentiate between the two. 

Beard, I think we should be able to just add a simple compile/link test using
'-framework Carbon' and set the define if the link succeeds.

 

Comment 5

[hacker formerly known as [e-mail]]

Attachment: Use presence of Carbon to denote OSX-ness

If the compile & link succeeds, we set OS_TARGET=MACOSX and add the define
-DXP_MACOSX=1.	We probably want to put something other than 'return 0;' in the
test prog though.

Comment 6

[Patrick C. Beard]

Yes, Chris has it exactly right. Darwin is a standalone FreeBSD-derived kernel,
which won't have the Carbon frameworks available. One could also simply test for
the existence of the file:

/System/Library/Frameworks/Carbon.framework

Comment 7

[hacker formerly known as [e-mail]]

Sure, you can make that assumption too.  I figured that if you're going to link
against it, you'd want to make sure that it worked before you started building.
 Anyway, your call.

Comment 8

[Wan-Teh Chang]

Patrick, could you have someone review your patch?  Thanks.

Comment 9

[David Hyatt]

r=hyatt

Comment 10

[hacker formerly known as [e-mail]]

Attachment: Combined patch that checks for the framework file

Comment 11

[hacker formerly known as [e-mail]]

Attachment: Fix classic mac bustage

Comment 12

[Simon Fraser [no longer active]]

pr_LoadLibraryByPathname is a huge mess, and I think this would be a good time
to clean it up. Ideally, we should push the core of it down into 'md' code, and
remove all the #ifdefs. wtc?

-    PRLibrary *lm=NULL;
+q    PRLibrary *lm=NULL;

This looks like a finger fumble.

Comment 13

[hacker formerly known as [e-mail]]

Attachment: fix non-carbon classic builds

Comment 14

[Wan-Teh Chang]

Will a mach-o application still need to load CFM libraries
in 2003?

Which important browser plugins still don't have a mach-o
version?

Why don't we ask the browser plugin vendors to provide
mach-o browser plugins?

Comment 15

[Brian Nesse (gone)]

>Will a mach-o application still need to load CFM libraries in 2003?
Almost certainly.

>Which important browser plugins still don't have a mach-o version?
All of them. The MRJ Plugin is the only mach-o plugin that I know of.

Comment 16

[Peter Lubczynski]

Comment on attachment 101669 [details] [diff] [review]
fix non-carbon classic builds

r=peterl

Comment 17

[Wan-Teh Chang]

I plan to review this patch tomorrow.  I understand
the urgency of this patch and I apologize for my delay
in reviewing the patch.

Comment 18

[Wan-Teh Chang]

Comment on attachment 101669 [details] [diff] [review]
fix non-carbon classic builds

I found that linking libnspr4.dylib with
-framework Carbon -framework CoreFoundation
causes NSPR test programs to fail to start
up in a command-line console window on Mac
OS X 10.1.5 with the these error messages:

kCGErrorIllegalArgument : initCGDisplayState: cannot map display interlocks.
kCGErrorIllegalArgument : CGSNewConnection cannot get connection port
kCGErrorIllegalArgument : CGSNewConnection cannot get connection port
kCGErrorInvalidConnection : CGSGetEventPort: Invalid connection

NSPR test programs work fine on in a
command-line console window Mac OS X
10.2.1.

Unless the problem on Mac OS X 10.1.5 can
be resolved, I am afraid that I can't take
this patch.  It is important that we be able
to run NSPR and NSS test programs in a
command-line console windows on Mac OS X.

Comment 19

[Simon Fraser [no longer active]]

Also, all the non-debug printfs need to be #ifdeffed out.

Comment 20

[Wan-Teh Chang]

Attachment: fix non-carbon classic builds v2

I made some stylistic and build changes to attachment
101669 [details] [diff] [review].  All the debug printf statements have been
deleted.

I have some questions about the original patch.

1. In nsprpub/pr/src/linking/Makefile.in, we have:

+# on Darwin / Mac OS X link use flat #includes.
+ifeq ($(OS_ARCH)_$(OS_TARGET),Darwin_MACOSX)
+INCLUDES    += -I/Developer/Headers/FlatCarbon -I$(srcdir)/../md/mac/
+endif

What does that comment mean?

2. In nsprpub/pr/src/linking/prlink.c, we have

-	 memcpy(cName, fileSpec.name + 1, fileSpec.name[0]);
-	 cName[fileSpec.name[0]] = '\0';
+#if TARGET_CARBON
+	 p2cstrcpy(cName, fileSpec.name);
+#else	      
+	 strncpy(cName, p2cstr(fileSpec.name),sizeof(cName));
+#endif

Does this work with non-carbob classic builds?

3. Does this FIXME comment mean this patch is not
finished?

+	 if (err == noErr && lm->connection) {
+	     /* FIXME:	if we're a mach-o binary, need to wrap all CFM function
pointers. */
+	     /* will need a hash-table of already seen function pointers, etc.
*/
+	     lm->main = TV2FP(lm->main);
+	 }

4. In this code, it seems that the XP_MACOSX case needs to
set lm->dlh because _PR_InitLinker only sets
pr_exe_loadmap->dlh (under the code for XP_UNIX and
USE_MACH_DYLD).

+#if defined(XP_MAC) || defined(XP_MACOSX)
+    lm->connection  = pr_exe_loadmap ? pr_exe_loadmap->connection : 0;
+#else
     lm->dlh	     = pr_exe_loadmap ? pr_exe_loadmap->dlh : 0;
+#endif

So maybe this code should look like this?

+#if defined(XP_MAC)
+    lm->connection  = pr_exe_loadmap ? pr_exe_loadmap->connection : 0;
+#else
     lm->dlh	     = pr_exe_loadmap ? pr_exe_loadmap->dlh : 0;
+#endif

Comment 21

[Wan-Teh Chang]

Here is a simple test to demonstrate the startup
problem on Mac OS X 10.1.5 caused by linking with
the Carbon framework.

1. Save the following test program as hello.c.

-=-=-=-=-= hello.c -=-=-=-=-=
#include <stdio.h>

int main()
{
    printf("Hello world!\n");
    return 0;
}
-=-=-=-=-= hello.c -=-=-=-=-=

2. Compile and run it on Mac OS X 10.1.5. (Netscape
employees can telnet to hans.mcom.com or
makaila.mcom.com with your Unix account.)

Here is what I got.

hans:/u/wtc/tmp 328% cc hello.c
hans:/u/wtc/tmp 329% ./a.out
Hello world!
hans:/u/wtc/tmp 330% cc hello.c -framework Carbon
hans:/u/wtc/tmp 331% ./a.out
kCGErrorIllegalArgument : initCGDisplayState: cannot map display interlocks.
kCGErrorIllegalArgument : CGSNewConnection cannot get connection port
kCGErrorIllegalArgument : CGSNewConnection cannot get connection port
kCGErrorInvalidConnection : CGSGetEventPort: Invalid connection
hans:/u/wtc/tmp 332% uname -a
Darwin hans.nscp.aoltw.net 5.5 Darwin Kernel Version 5.5: Thu May 30 14:51:26 PD
T 2002; root:xnu/xnu-201.42.3.obj~1/RELEASE_PPC  Power Macintosh powerpc
hans:/u/wtc/tmp 333% cc -v
Reading specs from /usr/libexec/gcc/darwin/ppc/2.95.2/specs
Apple Computer, Inc. version gcc-934.3, based on gcc version 2.95.2 19991024 (re
lease)

Comment 22

[Simon Fraser [no longer active]]

I think linking with Carbon causes the framework to try to make a connection
with the window server at load time.

Comment 23

[hacker formerly known as [e-mail]]

> +# on Darwin / Mac OS X link use flat #includes.
> +ifeq ($(OS_ARCH)_$(OS_TARGET),Darwin_MACOSX)
> +INCLUDES    += -I/Developer/Headers/FlatCarbon -I$(srcdir)/../md/mac/
> +endif
> What does that comment mean?

"In /Developer/Headers/FlatCarbon are stub files for all public Mac OS 9 header
files.   These stub files redirect the compiler to the appropriate umbrella
header file or contain warnings if the API is not valid on Mac OS X. " 
http://developer.apple.com/techpubs/macosx/Essentials/SystemOverview/Umbrella/Linking_and_g_Guideline.html

> 2. In nsprpub/pr/src/linking/prlink.c, we have
> Does this work with non-carbob classic builds?

My non-carbon build compiled, ran and loaded plugins.  I didn't extensively test
it though.

> 3. Does this FIXME comment mean this patch is not finished?

No idea.  Beard?

Comment 24

[hacker formerly known as [e-mail]]

Attachment: Use --enable-os-target=MacOSX to denote carbon build

I converted the --enable-win32-target option to be general enough to use as
--enable-os-target.  The original target was added for cross-compiling to
mingw32 which doesn't work anymore (requires the patches from bug 134113).  The
new option can work for the old purpose as well but it requires that you use
uppercase WINNT & WIN95 instead of translating the input.

Comment 25

[Wan-Teh Chang]

I filed a bug report with Apple about the
startup problem in a remote text terminal.
The problem ID is 3085140.  The problem
title is "Apps linked with the Carbon
framework fail to start up in a remote text
terminal."

Comment 26

[Wan-Teh Chang]

Attachment: Merged latest changes from CHIMERA_M1_0_1_BRANCH

Please review and test this patch.  (This needs
to be tested in Carbon and non-Carbon Classic
builds.)

This patch is based on the prlink.c on the
CHIMERA_M1_0_1_BRANCH (rev. 3.51.2.3.2.1.16.2)
and Chris Seawood's changes to fix Classic Mac
build bustage.

I did some editing to minimize the diffs against
the trunk and the CHIMERA_M1_0_1_BRANCH.  I am
allowing the debug printf statements for now.
(In general NSPR does not print debug messages.)

The problem with starting up a text-mode app in
a remote text terminal has been resolved.  My
sources at Apple told me that we should link with
the CoreServices framework instead of Carbon.

Comment 27

[Wan-Teh Chang]

Comment on attachment 104505 [details] [diff] [review]
Merged latest changes from CHIMERA_M1_0_1_BRANCH

Patrick,

You changed NSFindSymbol to FinsSymbol.  Is this a
safe change for the Carbon and non-Carbon Classic
builds?

Comment 28

[Wan-Teh Chang]

Attachment: Use FindSymbol in Mach-O build and NSFindSymbol in CFM builds

The only difference between this patch and the previous patch
is that we only call FindSymbol in the Mach-O build (XP_MACOSX)
and continue to call NSFindSymbol in the Carbon and non-Carbon
CFM builds.

Comment 29

[hacker formerly known as [e-mail]]

Comment on attachment 104590 [details] [diff] [review]
Use FindSymbol in Mach-O build and NSFindSymbol in CFM builds

I see no obvious side-effects in my classic, cfm carbon & mach-o builds.
r=cls

Comment 30

[hacker formerly known as [e-mail]]

The patch has been checked into the NSPR trunk & CLIENT_BRANCH.

Comment 31

[Patrick C. Beard]

Please check for leaks, I think that CFDataCreateMutable() returns a
CFMutableDataRef that has initial refcount of 1, and that CFDictionaryAddValue()
adds another reference count. It looks to me that we should add a call to
CFRelease() after the call to CFDictionaryAddValue() in TV2FP().

Comment 32

[hacker formerly known as [e-mail]]

Attachment: Fix CFData leak

Comment 33

[Christopher Blizzard (:blizzard)]

drivers doesn't want this in on the 1.2 branch before the 1.2 release, it's too
high risk.  If you guys want to check it in post-1.2 on that branch for chimera
or something that's OK but I don't think we want it before 1.2.

Comment 34

[Patrick C. Beard]

Comment on attachment 105950 [details] [diff] [review]
Fix CFData leak

r=beard

Comment 35

[hacker formerly known as [e-mail]]

The fix has been checked into the NSPR trunk & the client branch.