Fix reader memory stack use and verify read_object fully clears its stack.

Make abbreviation in writer not assume equivalent nodes have equivalent addresses (abbreviate more than on input if triples are sorted correctly, e.g. abbrevate ntriples)


git-svn-id: http://svn.drobilla.net/serd/trunk@149 490d8e77-9747-427b-9fa3-0b8f29cee8a0

4 files changed, 73 insertions, 33 deletions

diff --git a/serd/serd.h b/serd/serd.h
index 6865a1c7..91dbb15c 100644
--- a/serd/serd.h
+++ b/serd/serd.h
@@ -298,6 +298,13 @@ SerdNode
 serd_node_copy(const SerdNode* node);
 
 /**
+   Return true iff @a a is equal to @a b.
+*/
+SERD_API
+bool
+serd_node_equals(const SerdNode* a, const SerdNode* b);
+
+/**
    Simple wrapper for serd_node_new_uri to resolve a URI node.
 */
 SERD_API
diff --git a/src/node.c b/src/node.c
index 5901f159..173c5358 100644
--- a/src/node.c
+++ b/src/node.c
@@ -34,12 +34,26 @@ SerdNode
 serd_node_copy(const SerdNode* node)
 {
 	SerdNode copy = *node;
-	uint8_t* buf  = malloc(copy.n_bytes);
-	memcpy(buf, node->buf, copy.n_bytes);
-	copy.buf = buf;
+	if (node->buf) {
+		uint8_t* buf  = malloc(copy.n_bytes);
+		memcpy(buf, node->buf, copy.n_bytes);
+		copy.buf = buf;
+	}
 	return copy;
 }
 
+SERD_API
+bool
+serd_node_equals(const SerdNode* a, const SerdNode* b)
+{
+	return (a == b)
+		|| (a->type == b->type
+		    && a->n_bytes == b->n_bytes
+		    && a->n_chars == b->n_chars
+		    && ((a->buf == b->buf) || !strcmp((const char*)a->buf,
+		                                      (const char*)b->buf)));
+}
+
 static size_t
 serd_uri_string_length(const SerdURI* uri)
 {
diff --git a/src/reader.c b/src/reader.c
index a237bbf2..0f250a0c 100644
--- a/src/reader.c
+++ b/src/reader.c
@@ -204,21 +204,12 @@ stack_is_top_string(SerdReader reader, Ref ref)
 }
 #endif
 
-static inline intptr_t
-pad_size(intptr_t size)
-{
-	return (size + 7) & (~7);
-}
-
 // Make a new string from a non-UTF-8 C string (internal use only)
 static Ref
 push_string(SerdReader reader, const char* c_str, size_t n_bytes)
 {
-	// Align strings to 64-bits (assuming malloc/realloc are aligned to 64-bits)
-	const size_t stack_size = pad_size((intptr_t)reader->stack.size);
-	const size_t pad        = stack_size - reader->stack.size;
-	uint8_t*     mem        = serd_stack_push(
-		&reader->stack, pad + sizeof(SerdString) + n_bytes) + pad;
+	uint8_t* mem = serd_stack_push(&reader->stack,
+	                               sizeof(SerdString) + n_bytes);
 	SerdString* const str = (SerdString*)mem;
 	str->n_bytes = n_bytes;
 	str->n_chars = n_bytes - 1;
@@ -278,7 +269,7 @@ pop_string(SerdReader reader, Ref ref)
 		--reader->n_allocs;
 		#endif
 		SerdString* str = deref(reader, ref);
-		serd_stack_pop(&reader->stack, pad_size(sizeof(SerdString) + str->n_bytes));
+		serd_stack_pop(&reader->stack, sizeof(SerdString) + str->n_bytes);
 	}
 }
 
@@ -1061,6 +1052,10 @@ read_object(SerdReader reader, ReadContext ctx)
 	static const char* const XSD_BOOLEAN     = NS_XSD "boolean";
 	static const size_t      XSD_BOOLEAN_LEN = 40;
 
+#ifndef NDEBUG
+	const size_t orig_stack_size = reader->stack.size;
+#endif
+
 	uint8_t       pre[6];
 	bool          ret  = false;
 	bool          emit = (ctx.subject != 0);
@@ -1118,6 +1113,9 @@ except:
 	pop_string(reader, o.lang);
 	pop_string(reader, o.datatype);
 	pop_string(reader, o.value);
+#ifndef NDEBUG
+	assert(reader->stack.size == orig_stack_size);
+#endif
 	return ret;
 }
 
diff --git a/src/writer.c b/src/writer.c
index dd00d1d8..b87ad1b0 100644
--- a/src/writer.c
+++ b/src/writer.c
@@ -158,6 +158,18 @@ serd_writer_write_delim(SerdWriter writer, const uint8_t delim)
 	}
 }
 
+static void
+reset_context(SerdWriter writer)
+{
+	if (writer->context.graph.buf)
+		serd_node_free(&writer->context.graph);
+	if (writer->context.subject.buf)
+		serd_node_free(&writer->context.subject);
+	if (writer->context.predicate.buf)
+		serd_node_free(&writer->context.predicate);
+	writer->context = WRITE_CONTEXT_NULL;
+}
+	
 static bool
 write_node(SerdWriter      writer,
            const SerdNode* node,
@@ -175,9 +187,11 @@ write_node(SerdWriter      writer,
 			serd_writer_write_delim(writer, '[');
 			WriteContext* ctx = (WriteContext*)serd_stack_push(
 				&writer->anon_stack, sizeof(WriteContext));
-			*ctx = writer->context;
-			writer->context.subject   = *node;
-			writer->context.predicate = SERD_NODE_NULL;
+			ctx->graph     = serd_node_copy(&writer->context.graph);
+			ctx->subject   = serd_node_copy(&writer->context.subject);
+			ctx->predicate = serd_node_copy(&writer->context.predicate);
+			reset_context(writer);
+			writer->context.subject = serd_node_copy(node);
 			break;
 		}
 	case SERD_ANON:
@@ -285,13 +299,15 @@ serd_writer_write_statement(SerdWriter      writer,
 	case SERD_TURTLE:
 		break;
 	}
-	if (subject->buf == writer->context.subject.buf) {
-		if (predicate->buf == writer->context.predicate.buf) {  // Abbreviate S P
+	if (serd_node_equals(subject, &writer->context.subject)) {
+		if (serd_node_equals(predicate, &writer->context.predicate)) {
+			// Abbreviate S P
 			++writer->indent;
 			serd_writer_write_delim(writer, ',');
 			write_node(writer, object, object_datatype, object_lang);
 			--writer->indent;
-		} else {  // Abbreviate S
+		} else {
+			// Abbreviate S
 			if (writer->context.predicate.buf) {
 				serd_writer_write_delim(writer, ';');
 			} else {
@@ -299,7 +315,9 @@ serd_writer_write_statement(SerdWriter      writer,
 				serd_writer_write_delim(writer, '\n');
 			}
 			write_node(writer, predicate, NULL, NULL);
-			writer->context.predicate = *predicate;
+			if (writer->context.predicate.buf)
+				serd_node_free(&writer->context.predicate);
+			writer->context.predicate = serd_node_copy(predicate);
 			writer->sink(" ", 1, writer->stream);
 			write_node(writer, object, object_datatype, object_lang);
 		}
@@ -328,19 +346,21 @@ serd_writer_write_statement(SerdWriter      writer,
 			}
 		}
 
-		writer->context.subject   = *subject;
+		reset_context(writer);
+		writer->context.subject   = serd_node_copy(subject);
 		writer->context.predicate = SERD_NODE_NULL;
 
 		write_node(writer, predicate, NULL, NULL);
-		writer->context.predicate = *predicate;
+		writer->context.predicate = serd_node_copy(predicate);
 		writer->sink(" ", 1, writer->stream);
 
 		write_node(writer, object, object_datatype, object_lang);
 	}
 
-	const WriteContext new_context = { graph ? *graph : SERD_NODE_NULL,
-	                                   *subject,
-	                                   *predicate };
+	const WriteContext new_context = { graph ? serd_node_copy(graph) : SERD_NODE_NULL,
+	                                   serd_node_copy(subject),
+	                                   serd_node_copy(predicate) };
+	reset_context(writer);
 	writer->context = new_context;
 	return true;
 }
@@ -361,10 +381,11 @@ serd_writer_end_anon(SerdWriter      writer,
 	--writer->indent;
 	serd_writer_write_delim(writer, '\n');
 	writer->sink("]", 1, writer->stream);
+	reset_context(writer);
 	writer->context = *anon_stack_top(writer);
 	serd_stack_pop(&writer->anon_stack, sizeof(WriteContext));
 	if (!writer->context.subject.buf) {  // End of anonymous subject
-		writer->context.subject = *node;
+		writer->context.subject = serd_node_copy(node);
 	}
 	return true;
 }
@@ -375,8 +396,8 @@ serd_writer_finish(SerdWriter writer)
 {
 	if (writer->context.subject.buf) {
 		writer->sink(" .\n", 3, writer->stream);
-		writer->context.subject.buf = NULL;
 	}
+	reset_context(writer);
 }
 
 SERD_API
@@ -411,13 +432,13 @@ serd_writer_set_base_uri(SerdWriter     writer,
 	if (writer->syntax != SERD_NTRIPLES) {
 		if (writer->context.graph.buf || writer->context.subject.buf) {
 			writer->sink(" .\n\n", 4, writer->stream);
-			writer->context = WRITE_CONTEXT_NULL;
+			reset_context(writer);
 		}
 		writer->sink("@base <", 7, writer->stream);
 		serd_uri_serialise(uri, writer->sink, writer->stream);
 		writer->sink("> .\n", 4, writer->stream);
 	}
-	writer->context = WRITE_CONTEXT_NULL;
+	reset_context(writer);
 }
 
 SERD_API
@@ -429,7 +450,7 @@ serd_writer_set_prefix(SerdWriter      writer,
 	if (writer->syntax != SERD_NTRIPLES) {
 		if (writer->context.graph.buf || writer->context.subject.buf) {
 			writer->sink(" .\n\n", 4, writer->stream);
-			writer->context = WRITE_CONTEXT_NULL;
+			reset_context(writer);
 		}
 		writer->sink("@prefix ", 8, writer->stream);
 		writer->sink(name->buf, name->n_bytes - 1, writer->stream);
@@ -437,7 +458,7 @@ serd_writer_set_prefix(SerdWriter      writer,
 		write_text(writer, WRITE_URI, uri->buf, uri->n_bytes - 1, '>');
 		writer->sink("> .\n", 4, writer->stream);
 	}
-	writer->context = WRITE_CONTEXT_NULL;
+	reset_context(writer);
 	return true;
 }