Bug 169154

Summary: Add support for relative pathnames to JSC config files
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, commit-queue, darin, keith_miller, mark.lam, mcatanzaro, sbarati
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch sbarati: review+

Description Michael Saboff 2017-03-03 16:11:02 PST
There are two relative path support issues.

1) To allow for log file names relative to where the config file is located.
2) To determine the absolute path of the config file when a relative name is used.  This is needed so that issue #1 can be addressed.
Comment 1 Michael Saboff 2017-03-03 16:22:00 PST
<rdar://problem/30844619>
Comment 2 Michael Saboff 2017-03-03 16:22:37 PST
Created attachment 303355 [details]
Patch
Comment 3 Saam Barati 2017-03-03 16:37:36 PST
Comment on attachment 303355 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303355&action=review

r=me

> Source/JavaScriptCore/runtime/ConfigFile.cpp:434
> +            if (sizeof(filenameBuffer) - 1  > pathnameLength + shouldAddPathSeparator) {

Why not >=?
Comment 4 Michael Saboff 2017-03-03 16:45:27 PST
(In reply to comment #3)
> > Source/JavaScriptCore/runtime/ConfigFile.cpp:434
> > +            if (sizeof(filenameBuffer) - 1  > pathnameLength + shouldAddPathSeparator) {
> 
> Why not >=?

I think you're right, >= is fine.  I made that change.
Comment 5 Michael Saboff 2017-03-03 17:09:50 PST
Committed r213399: <http://trac.webkit.org/changeset/213399>
Comment 6 Carlos Alberto Lopez Perez 2017-03-03 21:01:57 PST
Committed r213416: <http://trac.webkit.org/changeset/213416>
Comment 7 Darin Adler 2017-03-04 10:16:52 PST
I don’t think we need an #if around include of <unistd.h>. Do we compile on any platform that does not have a header by that name?
Comment 8 Michael Catanzaro 2017-03-04 12:58:12 PST
Surely Windows does not have unistd.h? Is that wrong?
Comment 9 Darin Adler 2017-03-05 12:11:15 PST
(In reply to comment #8)
> Surely Windows does not have unistd.h? Is that wrong?

I had though we had some sort of compatibility version of <unistd.h> for Windows, but looking at the rest of our code it seems that we don’t.

(I see an unguarded include of that header in WebProcess.cpp, but I think that’s in code we don’t compile for Windows. I think there used to be a compatibility version of the header inside ANGLE, but not any more and we wouldn’t have wanted to rely on it anyway. Instead we include <unistd.h> in a few different places, each time with a different #if. So I guess this makes it no worse.)