Skip to content

Support reading script source from stdin in qjs #947

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

Merged
merged 1 commit into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion qjs.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ static int eval_buf(JSContext *ctx, const void *buf, int buf_len,
val = JS_Eval(ctx, buf, buf_len, filename,
eval_flags | JS_EVAL_FLAG_COMPILE_ONLY);
if (!JS_IsException(val)) {
use_realpath = (*filename != '<'); // ex. "<cmdline>"
// ex. "<cmdline>" pr "/dev/stdin"
use_realpath =
!(*filename == '<' || !strncmp(filename, "/dev/", 5));
if (js_module_set_import_meta(ctx, val, use_realpath, true) < 0) {
js_std_dump_error(ctx);
ret = -1;
Expand Down
60 changes: 26 additions & 34 deletions quickjs-libc.c
Original file line number Diff line number Diff line change
Expand Up @@ -418,45 +418,37 @@ static JSValue js_printf_internal(JSContext *ctx,
uint8_t *js_load_file(JSContext *ctx, size_t *pbuf_len, const char *filename)
{
FILE *f;
uint8_t *buf;
size_t buf_len;
long lret;
size_t n, len;
uint8_t *p, *buf, tmp[8192];

f = fopen(filename, "rb");
if (!f)
return NULL;
if (fseek(f, 0, SEEK_END) < 0)
goto fail;
lret = ftell(f);
if (lret < 0)
goto fail;
/* XXX: on Linux, ftell() return LONG_MAX for directories */
if (lret == LONG_MAX) {
errno = EISDIR;
goto fail;
}
buf_len = lret;
if (fseek(f, 0, SEEK_SET) < 0)
goto fail;
if (ctx)
buf = js_malloc(ctx, buf_len + 1);
else
buf = malloc(buf_len + 1);
if (!buf)
goto fail;
if (fread(buf, 1, buf_len, f) != buf_len) {
errno = EIO;
if (ctx)
js_free(ctx, buf);
else
free(buf);
fail:
fclose(f);
return NULL;
}
buf[buf_len] = '\0';
buf = NULL;
len = 0;
do {
n = fread(tmp, 1, sizeof(tmp), f);
Comment on lines +429 to +430
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.

if (ctx) {
p = js_realloc(ctx, buf, len + n + 1);
} else {
p = realloc(buf, len + n + 1);
}
if (!p) {
if (ctx) {
js_free(ctx, buf);
} else {
free(buf);
}
fclose(f);
return NULL;
}
memcpy(&p[len], tmp, n);
buf = p;
len += n;
buf[len] = '\0';
} while (n == sizeof(tmp));
fclose(f);
*pbuf_len = buf_len;
*pbuf_len = len;
return buf;
}

Expand Down