Skip to content

Conversation

@bptato
Copy link
Contributor

@bptato bptato commented Sep 30, 2025

If it "takes ownership of |values|", it must also free them on exception.

If it "takes ownership of |values|", it must also free them on exception.
@bptato bptato marked this pull request as draft September 30, 2025 16:07
@bptato
Copy link
Contributor Author

bptato commented Sep 30, 2025

Ah but that adds a double free to OP_array_from doesn't it...
What to do? Change the API or just document the quirk?

@bnoordhuis
Copy link
Contributor

That's a good point but, untested, I think you can just do this:

diff --git a/quickjs.c b/quickjs.c
index 63786a3..ecdcdda 100644
--- a/quickjs.c
+++ b/quickjs.c
@@ -16636,9 +16636,9 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
                 pc += 2;
                 call_argv = sp - call_argc;
                 ret_val = JS_NewArrayFrom(ctx, call_argc, call_argv);
+                sp -= call_argc;
                 if (unlikely(JS_IsException(ret_val)))
                     goto exception;
-                sp -= call_argc;
                 *sp++ = ret_val;
             }
             BREAK;

@bptato
Copy link
Contributor Author

bptato commented Sep 30, 2025

Right, that should work, thanks.

@bptato bptato marked this pull request as ready for review September 30, 2025 16:28
@bnoordhuis bnoordhuis merged commit 76b3b12 into quickjs-ng:master Oct 1, 2025
127 checks passed
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.

3 participants