From e5448af6d8ee1404e16fa55ef21a14869b1f54dc Mon Sep 17 00:00:00 2001
From: Rodrigo Arias <rodrigo.arias@bsc.es>
Date: Fri, 6 Sep 2024 14:56:13 +0200
Subject: [PATCH] Rebuild ring buffer pointers after injecting events

The ring buffer pointers are no longer valid as we may have displaced
events around. The pointers of the affected region are reconstructed by
reading the events again, following their size.
---
 CHANGELOG.md       |  4 ++++
 src/emu/ovnisort.c | 40 +++++++++++++++++++++++++++++++---------
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 807e9e7..ef8b274 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Enable -Wconversion and -Wsign-conversion.
 
+### Fixed
+
+- Fix bug in ovnisort when injecting events in a previously modified section.
+
 ## [1.10.0] - 2024-07-26
 
 ### Changed
diff --git a/src/emu/ovnisort.c b/src/emu/ovnisort.c
index ac15673..8f621b9 100644
--- a/src/emu/ovnisort.c
+++ b/src/emu/ovnisort.c
@@ -75,11 +75,10 @@ ring_add(struct ring *r, struct ovni_ev *ev)
 }
 
 static void
-ring_check(struct ring *r, long long from)
+ring_check(struct ring *r, long long start)
 {
 	uint64_t last_clock = 0;
-	long long start = from != -1 ? from : r->head;
-	for (long long i = start; i != r->tail; i = i + 1 >= r->size ? 0 : i + 1) {
+	for (long long i = start; i != r->tail; i = (i + 1) % r->size) {
 		uint64_t clock = r->ev[i]->header.clock;
 		if (clock < last_clock) {
 			die("ring not sorted at i=%lld, last_clock=%"PRIu64" clock=%"PRIu64 ,
@@ -281,6 +280,32 @@ write_stream(int fd, void *base, void *dst, const void *src, size_t size)
 	}
 }
 
+static void
+rebuild_ring(struct ring *r, long long start, struct ovni_ev *first, struct ovni_ev *last)
+{
+	long long nbad = 0;
+	long long n = 0;
+	struct ovni_ev *ev = first;
+
+	for (long long i = start; i != r->tail; i = i + 1 >= r->size ? 0 : i + 1) {
+		n++;
+		if (ev != r->ev[i])
+			nbad++;
+
+		if (ev >= last)
+			die("exceeding last pointer");
+
+		r->ev[i] = ev;
+		size_t size = ovni_ev_size(ev);
+		ev = (struct ovni_ev *) (((uint8_t *) ev) + size);
+	}
+
+	if (ev != last)
+		die("inconsistency: ev != last");
+
+	dbg("rebuilt ring with %lld / %lld misplaced events", nbad, n);
+}
+
 static int
 execute_sort_plan(struct sortplan *sp)
 {
@@ -322,6 +347,9 @@ execute_sort_plan(struct sortplan *sp)
 
 	free(buf);
 
+	/* Pointers from the ring buffer are invalid now, rebuild them */
+	rebuild_ring(sp->r, dirty, first, sp->next);
+
 	/* Invariant: The ring buffer is always sorted here. Check from the
 	 * dirty position onwards, so we avoid scanning all events. */
 	ring_check(sp->r, dirty);
@@ -386,12 +414,6 @@ stream_winsort(struct stream *stream, struct ring *r)
 			}
 		}
 
-		/* FIXME: After executing a sort plan, we may have pointers that
-		 * now point to garbage, as the events have moved. We can test
-		 * this by first sorting a given region so that it displaces the
-		 * offsets of the end marker, and then injecting events that are
-		 * inside the first unsorted region. Hopefully, trying to access
-		 * the clock of the end marker will now read garbage. */
 		ring_add(r, ev);
 	}