Bug 169154 - Add support for relative pathnames to JSC config files
Summary: Add support for relative pathnames to JSC config files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-03-03 16:11 PST by Michael Saboff
Modified: 2017-03-05 12:11 PST (History)
7 users (show)

See Also:


Attachments
Patch (5.22 KB, patch)
2017-03-03 16:22 PST, Michael Saboff
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.)