Last Comment Bug 171883 - xp_path.h can be removed
: xp_path.h can be removed
Product: SeaMonkey
Classification: Client Software
Component: Build Config (show other bugs)
: Trunk
: All All
P5 trivial (vote)
: mozilla1.4beta
Assigned To: hacker formerly known as
: Jon Granrose
Depends on:
Blocks: 199920
  Show dependency treegraph
Reported: 2002-10-01 02:37 PDT by Jörg Brunsmann
Modified: 2004-11-22 17:25 PST (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

reduce usage of SEPARATORs to minimum (2.04 KB, patch)
2002-10-05 05:03 PDT, Jörg Brunsmann
no flags Details | Diff | Splinter Review
Add PR_GetPathSeparator (1.35 KB, patch)
2003-03-31 22:21 PST, hacker formerly known as
no flags Details | Diff | Splinter Review
bye xp_path.h (2.08 KB, patch)
2003-03-31 22:23 PST, hacker formerly known as
bryner: review+
alecf: superreview+
Details | Diff | Splinter Review
Update .def file too (1.81 KB, patch)
2003-04-03 16:34 PST, hacker formerly known as
wtc: review-
Details | Diff | Splinter Review
Corrected description (1.78 KB, patch)
2003-04-03 18:10 PST, hacker formerly known as
wtc: review+
Details | Diff | Splinter Review

Description User image Jörg Brunsmann 2002-10-01 02:37:32 PDT
The file

is included only in two places. First, ut is included in


where the macros of xp_path are not used. Also, 
xp_path.h is included in


where some macros are used. But these macros
can be exchanged by the equivalent definitions
from NSPR. After that, xp_path.h can be deleted.
Comment 1 User image hacker formerly known as 2002-10-01 06:31:00 PDT
IIRC, the NSPR macros that xp_path.h duplicates are not available from public
headers so the file itself cannot be removed if people are still using it.
Comment 2 User image Jörg Brunsmann 2002-10-01 07:12:34 PDT
Adding to cc list.
jkeiser: is it possible for you to delete this line
Comment 3 User image Jörg Brunsmann 2002-10-01 07:33:13 PDT
There are possibilities:

1. xp_path.h defines system dependent path and directory separator. The 
directory separator is available here

The function NSPR_API(char) PR_GetPathSeparator(void) is missing, but could be 

2. Move the SEPARATOR definitions from xp_path.h to .

That this is no problem, can be verified by searching for 
SEPARATOR to see that other .cpp-files define their own SEPARATOR characters. 
Comment 4 User image hacker formerly known as 2002-10-01 09:44:51 PDT
1) You're talking about changing the NSPR API to expose that extra function. 
You would have to get buy-in from the NSPR owners for that.
2) This is the easiest solution but not the correct one.  I see _SEPARATOR being
defined in a standalone programs (nsinstall) and standalone libraries (nspr,
xpcom & js).  OJI is neither; it should be using the definitions from one of the
previously mentioned libraries since it depends upon them all.

Comment 5 User image Wan-Teh Chang 2002-10-01 10:36:53 PDT
It seems fine to add a new PR_GetPathSeparator function.
I am worried that that function is not that useful
because the directory and path separators NSPR uses on
the Mac are Unix-centric and may not be what you want.
Comment 6 User image John Keiser (jkeiser) 2002-10-01 12:15:37 PDT
It is most likely possible to remove that #include, yes.
Comment 7 User image Jörg Brunsmann 2002-10-01 13:37:53 PDT
Ok, there's consensus for a new nspr function.

If I understand correctly the definitions found in directory


for the files _beos.h, _os2.h, _macos.h and _winnt.h

match those unix-centric mac definitions found in xp_path.h

jkeiser: can you or someone else please remove the #include. Thanks.
Comment 8 User image John Keiser (jkeiser) 2002-10-02 02:02:48 PDT
Such a new function would be good too, IIRC we do funky platform-specific path
stuff in nsLocalFile.

As to removing #include xp_path.h, wouldn't it be simpler if whoever removes the
file also removes the include?  I don't really see the need to go through two
separate write/build/r=/sr=/commit cycles for what is essentially a single
patch.  If I'm missing some crucial element here, let me know.
Comment 9 User image Jörg Brunsmann 2002-10-02 04:54:11 PDT
The semantic of path and directory separator differ in xp_path.h and .

Furthermore, xp_path.h defines both PR_PATH_[DIRECTORY|SEPARATOR]_STR and

My understanding is as follows: a directory separator separates directory names
in absolute file names (in java: file.separator). A path separator separates
several directories (in java: path.separator) in a complete path specification.
The names DIRECTORY_SEPARATOR and PATH_SEPARATOR may be disleading.
FILE_SEPARATOR and PATH_SEPARATOR might be a better solution. What is your

Also there's a difference in the definition in MAXPATHLEN for XP_MAC: it's 512
in xp_path.h and 31 in nsLocalFileCommon.cpp. Perhaps an additional function
NSPR_API(int) PR_GetMaxPathLen(void) makes sense.

Being pragmatic I suggest to leave out the definitions in nsLocalFileCommon.cpp
for now and open another bug for merging all definitions of SEPARATORs and
PATHLENs and just pay attention to get rid of xp_path.h
Comment 10 User image Jörg Brunsmann 2002-10-05 05:00:19 PDT
Goal: get rid of xp_path.h
Barrier: /modules/oji/src/nsJVMManager.cpp
Pragmatic solution: Upcoming patch

Comment 11 User image Jörg Brunsmann 2002-10-05 05:03:02 PDT
Created attachment 101812 [details] [diff] [review]
reduce usage of SEPARATORs to minimum
Comment 12 User image hacker formerly known as 2002-10-08 12:03:08 PDT
Comment on attachment 101812 [details] [diff] [review]
reduce usage of SEPARATORs to minimum

Why are you commenting out the GetClasspathAdditions function?
Comment 13 User image Jörg Brunsmann 2002-10-08 12:32:42 PDT
I commented out the GetClasspathAdditions function because it's not referenced. 
And since this function uses the PR_PATH_SEPARATOR preprocessor macro, deleting 
this function would reduce the usage of the macros defined in xp_path.h to only 
one macro (PR_DIRECTORY_SEPARATOR). If removing this function is not wanted, 
one has to define the macro PR_PATH_SEPARATOR as well in nsJVMManager.cpp. Both 
ways are ok, since one can still remove xp_path.h.

Someone with deeper knowledge of nsJVMManager.cpp has to decide what to do, 
since it's the only file which uses xp_path.h. This person might also detect 
whether the nsJVMManager.cpp code will work on a mac given the unix-style 
SEPARATOR definitions from xp_path.h or if some conversions (see
should apply, like NSPR code does.

BTW: The '#ifdef 0' in the patch should read '#if 0' .
Comment 14 User image Joe Chou 2002-10-08 16:38:07 PDT
Checked with the Java plugin group to see if xp_path.h was referenced in Java
plugin code. The answer is No.
Comment 15 User image hacker formerly known as 2002-10-08 17:56:37 PDT
GetClasspathAdditions may not be directly referenced by any Mozilla code but
since it *is* part of the exported interface, it may be used by 3rd party
modules (in particular, the java plugin_.  Removing an interface function just
because it references an otherwise inconsequential define seems like an
extremely bad idea.  We don't need to get rid of xp_path.h that badly.

Comment 16 User image Joe Chou 2002-10-09 12:17:31 PDT
Currently, xp_path.h is used in oji module (some separators defined in xp_path.h
are used in nsJVMManager.cpp). OJI is being re-designed at the moment, so untill
the re-design is done,I don't think it would be a good idea to make any changes
to make removing xp_path.h possible. 
Ccing Patrick to see if any implication on Mac's Java plugin as well.
Comment 17 User image Christian :Biesinger (don't email me, ping me on IRC) 2002-10-10 10:32:15 PDT
also see bug 173634
Comment 18 User image Brendan Eich [:brendan] 2002-10-10 11:28:24 PDT
Can we pull this in to 1.3?

Comment 19 User image hacker formerly known as 2003-03-31 22:21:49 PST
Created attachment 119038 [details] [diff] [review]
Add PR_GetPathSeparator
Comment 20 User image hacker formerly known as 2003-03-31 22:23:51 PST
Created attachment 119039 [details] [diff] [review]
bye xp_path.h
Comment 21 User image Alec Flett 2003-04-01 22:40:54 PST
Comment on attachment 119039 [details] [diff] [review]
bye xp_path.h

Comment 22 User image hacker formerly known as 2003-04-03 16:34:39 PST
Created attachment 119373 [details] [diff] [review]
Update .def file too
Comment 23 User image Wan-Teh Chang 2003-04-03 17:48:26 PST
Comment on attachment 119373 [details] [diff] [review]
Update .def file too

>+** Get the host' path separator.
>+**  Pathnames are then assumed to be of the form:
>+**      [<sep><root_component><sep>]*(<component><sep>)<leaf_name>
>+NSPR_API(char) PR_GetPathSeparator(void);

The comment was copied from PR_GetDirectorySeparator.  It needs
to be rewritten for PR_GetPathSeparator.

Everything else is good.
Comment 24 User image hacker formerly known as 2003-04-03 18:10:21 PST
Created attachment 119381 [details] [diff] [review]
Corrected description

Comment 25 User image Wan-Teh Chang 2003-04-03 18:53:48 PST
Comment on attachment 119381 [details] [diff] [review]
Corrected description

Comment 26 User image hacker formerly known as 2003-04-03 21:04:54 PST
The patches have been checked in on the mozilla trunk, nspr client branch & nspr

Note You need to log in before you can comment on or make changes to this bug.