Use a simple type-safe stack in writer

7 files changed, 94 insertions, 51 deletions

diff --git a/include/serd/world.h b/include/serd/world.h
index fe381628..b4849fa2 100644
--- a/include/serd/world.h
+++ b/include/serd/world.h
@@ -25,6 +25,7 @@ typedef struct SerdWorldImpl SerdWorld;
 /// Resource limits to control allocation
 typedef struct {
   size_t reader_stack_size;
+  size_t writer_max_depth;
 } SerdLimits;
 
 /**
diff --git a/meson/suppressions/meson.build b/meson/suppressions/meson.build
index 2ad63b00..88eb9f99 100644
--- a/meson/suppressions/meson.build
+++ b/meson/suppressions/meson.build
@@ -15,7 +15,6 @@ if is_variable('cc')
   if cc.get_id() in ['clang', 'emscripten']
     if warning_level == 'everything'
       c_suppressions += [
-        '-Wno-cast-align',
         '-Wno-cast-function-type-strict',
         '-Wno-declaration-after-statement',
         '-Wno-format-nonliteral',
@@ -52,7 +51,6 @@ if is_variable('cc')
   elif cc.get_id() == 'gcc'
     if warning_level == 'everything'
       c_suppressions += [
-        '-Wno-cast-align',
         '-Wno-format-nonliteral',
         '-Wno-inline',
         '-Wno-padded',
diff --git a/src/serdi.c b/src/serdi.c
index 105bda46..75599ac7 100644
--- a/src/serdi.c
+++ b/src/serdi.c
@@ -32,6 +32,8 @@
 #define SERDI_ERROR(msg) fprintf(stderr, "serdi: " msg)
 #define SERDI_ERRORF(fmt, ...) fprintf(stderr, "serdi: " fmt, __VA_ARGS__)
 
+#define MAX_DEPTH 128U
+
 static int
 print_version(void)
 {
@@ -245,7 +247,7 @@ main(int argc, char** argv)
   SerdWriter* const writer = serd_writer_new(
     world, output_syntax, writer_flags, env, (SerdWriteFunc)fwrite, out_fd);
 
-  const SerdLimits limits = {stack_size};
+  const SerdLimits limits = {stack_size, MAX_DEPTH};
   serd_world_set_limits(world, limits);
 
   SerdReader* const reader =
diff --git a/src/stack.h b/src/stack.h
index 6ad3385d..cdf2b0f6 100644
--- a/src/stack.h
+++ b/src/stack.h
@@ -33,12 +33,6 @@ serd_stack_new(size_t size)
   return stack;
 }
 
-static inline bool
-serd_stack_is_empty(const SerdStack* stack)
-{
-  return stack->size <= SERD_STACK_BOTTOM;
-}
-
 static inline void
 serd_stack_free(SerdStack* stack)
 {
diff --git a/src/world.c b/src/world.c
index 7fb5eea2..dcf0b6af 100644
--- a/src/world.c
+++ b/src/world.c
@@ -104,6 +104,7 @@ serd_world_new(void)
   }
 
   world->limits.reader_stack_size = 1048576U;
+  world->limits.writer_max_depth  = 128U;
   world->blank_node               = blank_node;
 
   return world;
diff --git a/src/writer.c b/src/writer.c
index 58fd0d53..5603e9ca 100644
--- a/src/writer.c
+++ b/src/writer.c
@@ -6,7 +6,6 @@
 #include "node.h"
 #include "serd_internal.h"
 #include "sink.h"
-#include "stack.h"
 #include "string_utils.h"
 #include "system.h"
 #include "try.h"
@@ -134,7 +133,9 @@ struct SerdWriterImpl {
   SerdEnv*        env;
   SerdNode*       root_node;
   SerdURIView     root_uri;
-  SerdStack       anon_stack;
+  WriteContext*   anon_stack;
+  size_t          max_depth;
+  size_t          anon_stack_size;
   SerdByteSink    byte_sink;
   WriteContext    context;
   char*           bprefix;
@@ -223,12 +224,12 @@ push_context(SerdWriter* const        writer,
              const SerdNode* const    predicate)
 {
   // Push the current context to the stack
-  void* const top = serd_stack_push(&writer->anon_stack, sizeof(WriteContext));
-  if (!top) {
+
+  if (writer->anon_stack_size >= writer->max_depth) {
     return SERD_BAD_STACK;
   }
 
-  *(WriteContext*)top = writer->context;
+  writer->anon_stack[writer->anon_stack_size++] = writer->context;
 
   // Update the current context
 
@@ -247,14 +248,10 @@ push_context(SerdWriter* const        writer,
 static void
 pop_context(SerdWriter* writer)
 {
-  // Replace the current context with the top of the stack
-  free_context(&writer->context);
-  writer->context =
-    *(WriteContext*)(writer->anon_stack.buf + writer->anon_stack.size -
-                     sizeof(WriteContext));
+  assert(writer->anon_stack_size > 0);
 
-  // Pop the top of the stack away
-  serd_stack_pop(&writer->anon_stack, sizeof(WriteContext));
+  free_context(&writer->context);
+  writer->context = writer->anon_stack[--writer->anon_stack_size];
 }
 
 SERD_NODISCARD static size_t
@@ -678,7 +675,7 @@ write_sep(SerdWriter* writer, const SerdStatementFlags flags, Sep sep)
 static void
 free_anon_stack(SerdWriter* writer)
 {
-  while (!serd_stack_is_empty(&writer->anon_stack)) {
+  while (writer->anon_stack_size > 0) {
     pop_context(writer);
   }
 }
@@ -706,6 +703,7 @@ reset_context(SerdWriter* writer, const unsigned flags)
     writer->indent = 0;
   }
 
+  writer->anon_stack_size        = 0;
   writer->context.type           = CTX_NAMED;
   writer->context.predicates     = false;
   writer->context.comma_indented = false;
@@ -1093,29 +1091,26 @@ serd_writer_write_statement(SerdWriter* const          writer,
 
   } else {
     // No abbreviation
-    if (serd_stack_is_empty(&writer->anon_stack)) {
-      if (ctx(writer, SERD_SUBJECT)) {
-        TRY(st, write_sep(writer, flags, SEP_END_S));
-      }
 
-      if (writer->last_sep == SEP_END_S || writer->last_sep == SEP_END_DIRECT) {
-        TRY(st, write_top_level_sep(writer));
-      }
+    if (ctx(writer, SERD_SUBJECT)) {
+      TRY(st, write_sep(writer, flags, SEP_END_S));
+    }
 
-      TRY(st, write_node(writer, subject, SERD_SUBJECT, flags));
-      if ((flags & (SERD_ANON_S | SERD_LIST_S))) {
-        TRY(st, write_sep(writer, flags, SEP_ANON_S_P));
-      } else {
-        TRY(st, write_sep(writer, flags, SEP_S_P));
-      }
+    if (writer->last_sep == SEP_END_S || writer->last_sep == SEP_END_DIRECT) {
+      TRY(st, write_top_level_sep(writer));
+    }
 
-    } else {
+    // Write subject node
+    TRY(st, write_node(writer, subject, SERD_SUBJECT, flags));
+    if (!(flags & (SERD_ANON_S | SERD_LIST_S))) {
+      TRY(st, write_sep(writer, flags, SEP_S_P));
+    } else if (flags & SERD_ANON_S) {
       TRY(st, write_sep(writer, flags, SEP_ANON_S_P));
     }
 
+    // Set context to new subject and write predicate
     reset_context(writer, 0U);
     serd_node_set(&writer->context.subject, subject);
-
     if (!(flags & SERD_LIST_S)) {
       TRY(st, write_pred(writer, flags, predicate));
     }
@@ -1158,7 +1153,7 @@ serd_writer_end_anon(SerdWriter* writer, const SerdNode* node)
     return SERD_SUCCESS;
   }
 
-  if (serd_stack_is_empty(&writer->anon_stack)) {
+  if (!writer->anon_stack_size) {
     return w_err(writer, SERD_BAD_EVENT, "unexpected end of anonymous node\n");
   }
 
@@ -1214,20 +1209,25 @@ serd_writer_new(SerdWorld*      world,
                 SerdWriteFunc   ssink,
                 void*           stream)
 {
-  const WriteContext context = WRITE_CONTEXT_NULL;
-  SerdWriter*        writer  = (SerdWriter*)calloc(1, sizeof(SerdWriter));
-
-  writer->world      = world;
-  writer->syntax     = syntax;
-  writer->flags      = flags;
-  writer->env        = env;
-  writer->root_node  = NULL;
-  writer->root_uri   = SERD_URI_NULL;
-  writer->anon_stack = serd_stack_new(SERD_PAGE_SIZE);
-  writer->context    = context;
-  writer->byte_sink  = serd_byte_sink_new(
+  const size_t       max_depth = world->limits.writer_max_depth;
+  const WriteContext context   = WRITE_CONTEXT_NULL;
+  SerdWriter*        writer    = (SerdWriter*)calloc(1, sizeof(SerdWriter));
+
+  writer->world     = world;
+  writer->syntax    = syntax;
+  writer->flags     = flags;
+  writer->env       = env;
+  writer->root_node = NULL;
+  writer->root_uri  = SERD_URI_NULL;
+  writer->context   = context;
+  writer->byte_sink = serd_byte_sink_new(
     ssink, stream, (flags & SERD_WRITE_BULK) ? SERD_PAGE_SIZE : 1);
 
+  if (max_depth) {
+    writer->max_depth  = max_depth;
+    writer->anon_stack = (WriteContext*)calloc(max_depth, sizeof(WriteContext));
+  }
+
   writer->iface.handle   = writer;
   writer->iface.on_event = (SerdEventFunc)serd_writer_on_event;
 
@@ -1328,7 +1328,7 @@ serd_writer_free(SerdWriter* writer)
   serd_writer_finish(writer);
   free_context(&writer->context);
   free_anon_stack(writer);
-  serd_stack_free(&writer->anon_stack);
+  free(writer->anon_stack);
   free(writer->bprefix);
   serd_byte_sink_free(&writer->byte_sink);
   serd_node_free(writer->root_node);
diff --git a/test/test_writer.c b/test/test_writer.c
index 9b885dd6..1334bb02 100644
--- a/test/test_writer.c
+++ b/test/test_writer.c
@@ -247,6 +247,52 @@ test_write_error(void)
   serd_world_free(world);
 }
 
+static void
+test_writer_stack_overflow(void)
+{
+  SerdWorld* world = serd_world_new();
+  SerdEnv*   env   = serd_env_new(serd_empty_string());
+
+  SerdWriter* writer =
+    serd_writer_new(world, SERD_TURTLE, 0U, env, null_sink, NULL);
+
+  const SerdSink* sink = serd_writer_sink(writer);
+
+  SerdNode* const s = serd_new_uri(serd_string("http://example.org/s"));
+  SerdNode* const p = serd_new_uri(serd_string("http://example.org/p"));
+
+  SerdNode*  o  = serd_new_blank(serd_string("blank"));
+  SerdStatus st = serd_sink_write(sink, SERD_ANON_O, s, p, o, NULL);
+  assert(!st);
+
+  // Repeatedly write nested anonymous objects until the writer stack overflows
+  for (unsigned i = 0U; i < 512U; ++i) {
+    char buf[1024];
+    snprintf(buf, sizeof(buf), "b%u", i);
+
+    SerdNode* next_o = serd_new_blank(serd_string(buf));
+
+    st = serd_sink_write(sink, SERD_ANON_O, o, p, next_o, NULL);
+
+    serd_node_free(o);
+    o = next_o;
+
+    if (st) {
+      assert(st == SERD_BAD_STACK);
+      break;
+    }
+  }
+
+  assert(st == SERD_BAD_STACK);
+
+  serd_node_free(o);
+  serd_node_free(p);
+  serd_node_free(s);
+  serd_writer_free(writer);
+  serd_env_free(env);
+  serd_world_free(world);
+}
+
 int
 main(void)
 {
@@ -256,6 +302,7 @@ main(void)
   test_writer_cleanup();
   test_strict_write();
   test_write_error();
+  test_writer_stack_overflow();
 
   return 0;
 }