From 874638748b50f99acd4acbf54ec9663c06eb96ee Mon Sep 17 00:00:00 2001
From: Laria Carolin Chabowski <laria@laria.me>
Date: Sun, 20 Nov 2022 21:42:46 +0100
Subject: [PATCH] Make iterative runner not panic on OOM

The iterative runner used functions that could throw errors on OOM before
protecting itself using apfl_call_protected. An error at that point would
have resulted in calling the panic handler as a last resort.

This now introduces the apfl_do_protected function which allows running C
code protected without needing to wrap it in a cfunc value, thus removing
the need for the functions that could throw in the iterative runner.
---
 src/context.c | 32 +++++++++++++++++++++++++++++---
 src/context.h |  7 +++++++
 src/eval.c    | 17 ++++-------------
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/src/context.c b/src/context.c
index f024396..af997fc 100644
--- a/src/context.c
+++ b/src/context.c
@@ -29,8 +29,12 @@ panic(apfl_ctx ctx, bool with_error_on_stack, enum apfl_result result)
 }
 
 enum apfl_result
-apfl_call_protected(apfl_ctx ctx, apfl_stackidx func, apfl_stackidx args, bool *with_error_on_stack)
-{
+apfl_do_protected(
+    apfl_ctx ctx,
+    void (*callback)(apfl_ctx, void *),
+    void *opaque,
+    bool *with_error_on_stack
+) {
     struct error_handler *prev_handler = ctx->error_handler;
 
     size_t callstack_len = ctx->call_stack.len;
@@ -42,7 +46,7 @@ apfl_call_protected(apfl_ctx ctx, apfl_stackidx func, apfl_stackidx args, bool *
     enum apfl_result result = APFL_RESULT_OK;
     int rv = setjmp(handler.jump);
     if (rv == 0) {
-        apfl_call(ctx, func, args);
+        callback(ctx, opaque);
     } else {
         result = (enum apfl_result)(rv - RESULT_OFF_FOR_LONGJMP);
         assert(result != APFL_RESULT_OK);
@@ -87,6 +91,28 @@ apfl_call_protected(apfl_ctx ctx, apfl_stackidx func, apfl_stackidx args, bool *
     return result;
 }
 
+struct call_protected_data {
+    apfl_stackidx func;
+    apfl_stackidx args;
+};
+
+static void
+call_protected_cb(apfl_ctx ctx, void *opaque)
+{
+    struct call_protected_data *data = opaque;
+    apfl_call(ctx, data->func, data->args);
+}
+
+enum apfl_result
+apfl_call_protected(apfl_ctx ctx, apfl_stackidx func, apfl_stackidx args, bool *with_error_on_stack)
+{
+    struct call_protected_data data = {
+        .func = func,
+        .args = args,
+    };
+    return apfl_do_protected(ctx, call_protected_cb, &data, with_error_on_stack);
+}
+
 APFL_NORETURN static void
 raise_error(apfl_ctx ctx, bool with_error_on_stack, enum apfl_result type)
 {
diff --git a/src/context.h b/src/context.h
index 200af96..18025ae 100644
--- a/src/context.h
+++ b/src/context.h
@@ -185,6 +185,13 @@ struct scope *apfl_closure_scope_for_func(apfl_ctx, struct scopes);
 
 struct apfl_format_writer apfl_get_output_writer(apfl_ctx);
 
+enum apfl_result apfl_do_protected(
+    apfl_ctx ctx,
+    void (*callback)(apfl_ctx, void *),
+    void *opaque,
+    bool *with_error_on_stack
+);
+
 bool apfl_ctx_register_iterative_runner(apfl_ctx, apfl_iterative_runner);
 void apfl_ctx_unregister_iterative_runner(apfl_ctx, apfl_iterative_runner);
 
diff --git a/src/eval.c b/src/eval.c
index 68a40c6..738355d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1471,11 +1471,10 @@ handle_parse_error(apfl_ctx ctx, struct apfl_error error)
 }
 
 static void
-iterative_runner_next_protected(apfl_ctx ctx)
+iterative_runner_next_protected(apfl_ctx ctx, void *opaque)
 {
-    apfl_stack_drop(ctx, -1);
-    apfl_cfunc_self_getslot(ctx, 0);
-    apfl_iterative_runner runner = apfl_get_userdata(ctx, -1);
+    (void)ctx;
+    apfl_iterative_runner runner = opaque;
 
     switch (apfl_parser_next(runner->parser)) {
     case APFL_PARSE_OK:
@@ -1501,15 +1500,7 @@ apfl_iterative_runner_next(apfl_iterative_runner runner)
 
     apfl_stack_clear(runner->ctx);
 
-    // TODO: It's a bit silly that we build the cfunc every time. Can we be
-    //       smarter and leave it on the stack somewhere? Or save it in the
-    //       runner? Also, what if the construction of the func fails? We're not
-    //       running protected yet :/
-    apfl_push_cfunc(runner->ctx, iterative_runner_next_protected, 1);
-    apfl_push_userdata(runner->ctx, runner);
-    apfl_cfunc_setslot(runner->ctx, -2, 0, -1);
-    apfl_list_create(runner->ctx, 0);
-    runner->result = apfl_call_protected(runner->ctx, -2, -1, &runner->with_error_on_stack);
+    runner->result = apfl_do_protected(runner->ctx, iterative_runner_next_protected, runner, &runner->with_error_on_stack);
 
     return !runner->end;
 }