Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check the given path in process.readSource #1359

Merged
merged 1 commit into from
Dec 12, 2017
Merged

Check the given path in process.readSource #1359

merged 1 commit into from
Dec 12, 2017

Conversation

knightburton
Copy link
Member

The path argument in the process.readSource must point
to an existing file, not a directory.
Fix #1351

IoT.js-DCO-1.0-Signed-off-by: Imre Kiss [email protected]

Copy link
Contributor

@glistening glistening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@glistening
Copy link
Contributor

glistening commented Dec 7, 2017

This comment is not about this PR.

Why we expose process.readSource publicly? (Although it is not documented in API reference, it is accessible from outside.) readSource is for reading js module's sources on module loading. I believe it must be private.

@@ -142,6 +142,14 @@ JS_FUNCTION(ReadSource) {
DJS_CHECK_ARGS(1, string);

iotjs_string_t path = JS_GET_ARG(0, string);

struct stat path_stat;
stat(iotjs_string_data(&path), &path_stat);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use the libtuv's stat call here?

The path argument in the process.readSource must point
to an existing file, not a directory.
Fix #1351

IoT.js-DCO-1.0-Signed-off-by: Imre Kiss [email protected]
@knightburton
Copy link
Member Author

So, I've updated the PR based on @galpeter review. @glistening please take a look to it.

@glistening
Copy link
Contributor

Still LGTM. ( @galpeter, thank you for comment. )

@yichoi
Copy link
Contributor

yichoi commented Dec 12, 2017

LGTM

@yichoi yichoi merged commit 6f896eb into jerryscript-project:master Dec 12, 2017
@knightburton knightburton deleted the assert-readscource branch March 19, 2018 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants