Skip to content

Conversation

bnoordhuis
Copy link
Contributor

Replace js_load_file with something that doesn't choke on special files and don't call realpath(3) on /dev files because that doesn't work.

Now echo 'print("hello")' | qjs /dev/stdin works.

Fixes: #945

Replace js_load_file with something that doesn't choke on special files
and don't call realpath(3) on /dev files because that doesn't work.

Fixes: quickjs-ng#945
@bnoordhuis bnoordhuis merged commit e2f61ac into quickjs-ng:master Mar 3, 2025
61 checks passed
@bnoordhuis bnoordhuis deleted the fix945 branch March 3, 2025 22:49
Comment on lines +429 to +430
do {
n = fread(tmp, 1, sizeof(tmp), f);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not use a regular loop while ((n = fread(tmp, 1, sizeof(tmp), f)) != 0) { ... }?
I the goal is to make sure buf is allocated for empty files, an explanatory comment would be welcome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't that self-evident though? do/while is for when a loop should execute at least once.

Copy link
Collaborator

@chqrlie chqrlie Mar 4, 2025

Choose a reason for hiding this comment

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

do/while loops are never self evident, IME they have a tendency to hide nasty bugs. Whenever you see one in code, read carefully and you might find a bug.
In this case, the loop runs at least once, and fread will return 0 on empty files, so the case of a 0 return must be handled gracefully. And as an extra bonus, the loop will run one extra time on file whose length is exactly a multiple of 8K. That's 2 corner cases to think about. With a regular while loop, there is no hidden special cases to take into consideration. Less brain space: unambiguous end test and explicit empty file test.

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.

[Feature Request] Option to execute script from stdin like nodejs

3 participants