327502 - use of setenv breaks solaris build

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Shane Caraveo]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

Patch 210974 from bug 312092 broke solaris builds due to use of setenv, rather than use of PR_SetEnv (as is used elsewhere in the same files).  

Reproducible: Always

Steps to Reproduce:

Actual Results:  
export/home/trentm/as/Mozilla-devel/build/cvs1.8-ko4.0-ko/mozilla/toolkit/xre/nsAppRunner.cpp: In
   function `int XRE_main(int, char**, const nsXREAppData*)':
/export/home/trentm/as/Mozilla-devel/build/cvs1.8-ko4.0-ko/mozilla/toolkit/xre/nsAppRunner.cpp:1871: error: `
   setenv' undeclared (first use this function)

Comment 1

[Trent Mick]

The breakage was on Solaris 8. I haven't tried on Solaris 10, but it looks like SOlaris 10's headers *do* define setenv() (guarded by __XOPEN_SOURCE, POSIX, et al.).

Perhaps using PR_SetEnv would be more appropriate. I'll attach a proposed patch.

Comment 2

[Trent Mick]

Attachment: use PR_SetEnv instead of setenv

Comment 3

[Trent Mick]

This likely also affects xpfe's nsAppRunner.cpp but we don't build that.

Comment 4

[Ginn Chen]

Solaris 10 is fine.
I didn't try Solaris 8 yet.

Comment 5

[Ginn Chen]

Firefox built OK with setenv on SunOS 5.8 Generic_108528-14 sun4u sparc with Sun C++ 5.5 Patch 113817-06 2004/01/29 

Comment 6

[Shane Caraveo]

Ginn, are you saying 5.8 built with setenv, not PR_SetEnv?  If so, I still beleive it is more correct to use the PR api's here.

Comment 7

[Ginn Chen]

Attachment: change to PR_GetEnv/PR_SetEnv

Comment 8

[Benjamin Smedberg]

Comment on attachment 212464 [details] [diff] [review]
change to PR_GetEnv/PR_SetEnv

Please change the
if (gtkModules)
to
if (gtkModules && gtkModules)

because with PR_GetEnv you're supposed to treat a null return and an empty string the same.

There's no need to use %s to insert GTK_MODULES, you can put it in the format string directly.

Comment 9

[Ginn Chen]

Attachment: patch v2

Comment 10

[Ginn Chen]

Checking in toolkit/xre/nsAppRunner.cpp;
/cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.134; previous revision: 1.133
done
Checking in xpfe/bootstrap/nsAppRunner.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.450; previous revision: 1.449
done

Comment 11

[Ginn Chen]

Checking in toolkit/xre/nsAppRunner.cpp;
/cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.113.2.9; previous revision: 1.113.2.8
done
Checking in xpfe/bootstrap/nsAppRunner.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.442.2.3; previous revision: 1.442.2.2
done