shithub: neoventi

Download patch

ref: 02baf1b7637be36e3666f5a71f210d23a1750917
parent: c370b53dcac6e4f542e0549e589e6e861d2590be
author: Noam Preil <noam@pixelhero.dev>
date: Wed Sep 17 23:10:16 EDT 2025

fix u16get, again

--- a/arena.c
+++ b/arena.c
@@ -4,9 +4,10 @@
 #include "neoventi.h"
 
 int
-arenarepair(VtArena *arena, char *ci, u64int addr)
+arenarepair(VtArena *arena, uchar *ci, u64int addr)
 {
 	uchar buf[30];
+	u16int tmp;
 	if(!vtarenaread(arena, addr, buf, 30)){
 		werrstr("arenarepair: %r");
 		return 0;
@@ -16,8 +17,11 @@
 		return 0;
 	}
 	ci[0] = 1;
-	U16PUT(ci + 1, U16GET(buf + 5));
+	fprint(2, "clump at addr %d / %d\n", U16GET(buf+5), U16GET(buf+7));
+	tmp = U16GET(buf+5);
+	U16PUT(ci + 1, tmp);
 	U16PUT(ci + 3, U16GET(buf + 7));
+	fprint(2, "clump at addr %d / %d\n", U16GET(ci+1), U16GET(ci+3));
 	return 1;
 }
 
@@ -53,17 +57,20 @@
 		fprint(2, "validating clump %d, offset %d, block %d\n", m, off, block);
 		off += ClumpInfoSize;
 		ci = &buf[off];
-		if(U32GET(ci) != arena->clumpmagic){
-			if(!arenarepair(arena, ci, addr)){
-				werrstr("clump %d needs repair, at address %lld: %r", m, addr);
+		if(ci[0] != 1){
+			fprint(2, "attempting repair of clump %d , at address %llud\n", m, addr);
+			if(!arenarepair(arena, (u8int*)ci, addr)){
+				werrstr("clump %d needs repair, at address %llud: %r", m, addr);
 				return 0;
 			}
 		}
-		addr += U16GET(ci + 1) + 38;
+
+		fprint(2, "addr %llud, adding u16 %ud + 38\n", addr, U16GET(ci+1));
+		addr += (((u64int)(U16GET(ci + 1))) + 38);
 	}
 	if(arena->arenastats.used != addr){
-		werrstr("corrupt: found addr %d, expected %d", addr, arena->arenastats.used);
-		return 0;
+		fprint(2, "corrupt: found addr %d, expected %d", addr, arena->arenastats.used);
+		arena->arenastats.used = addr;
 	}
 	return 1;
 }
--- a/disk.c
+++ b/disk.c
@@ -442,6 +442,7 @@
 		arena->arenastats.uncsize = U64GET(p+54);
 		// OR with indexstats: a bug from 2008 could leave arenastats out of date,
 		// and venti worked around it for ages so now I have to too.
+		// ...TODO: no, I don't.
 		arena->arenastats.sealed = U8GET(p+62) || arena->indexstats.sealed;
 	} else {
 		arena->arenastats = arena->indexstats;
--- a/neoventi.h
+++ b/neoventi.h
@@ -1,8 +1,8 @@
 #define	U8GET(p)	((p)[0])
-#define	U16GET(p)	((u16int)(((p)[0]<<8)|(p)[1]))
+#define	U16GET(p)	((u16int)(((u8int*)p)[0]<<8)|(((u8int*)p)[1]))
 #define	U32GET(p)	((u32int)(((p)[0]<<24)|((p)[1]<<16)|((p)[2]<<8)|(p)[3]))
 #define	U64GET(p)	(((u64int)U32GET(p)<<32)|(u64int)U32GET((p)+4))
-#define	U16PUT(p,v)	(p)[0]=(v)>>8;(p)[1]=(v)
+#define	U16PUT(p,v)	(p)[0]=(v)>>8;(p)[1]=(v&0xFF)
 #define	U32PUT(p,v)	(p)[0]=(v)>>24;(p)[1]=(v)>>16;(p)[2]=(v)>>8;(p)[3]=(v)
 #define	U64PUT(p,v,t32)	t32=(v)>>32;U32PUT(p,t32);t32=(v);U32PUT((p)+4,t32)
 
--- a/notebook
+++ b/notebook
@@ -3348,3 +3348,52 @@
 ...sign extension of what's supposed to be a u16?????? Is U16GET returning something not usable inline?
 
 Hahahahahahah was missing a cast in U16GET that was present in U32GET and U64GET, okay.
+
+
+addr 0, adding u16 65413 + 38
+validating clump 1, offset 25, block 8092
+attempting repair of clump 1 , at address 65451
+./6.neoventi: syncarenas: clump 1 needs repair, at address 65451: arenarepair: corrupt
+
+...that's... probably incorrect, too, given that block size isn't big enough to allow for that.
+
+We loaded the clump from the arena by address. It _should_ be correct. 
+
+Actually, sanity check from the trailer:
+
+Resuming: used 1527, block 0, blocksize 8192, offset 1527
+clumps: 5, 0
+validating clump 0, offset 0, block 8092
+attempting repair of clump 0 , at address 0
+clump at addr 389 / 389
+addr 0, adding u16 65413 + 38
+validating clump 1, offset 25, block 8092
+attempting repair of clump 1 , at address 65451
+
+We've got 5 clumps, with a total size of 1527 bytes, including 38*5 == 190 bytes of overhead.
+
+First clump says size is 389/389. Why are we adding 65413???
+
+...Is U16PUT also wrong? No. Pretty sure it's not. But.
+
+clump at addr 389 / 389
+clump at addr 65413 / 65413
+
+Reading it back shows a different result, so: yes, yes it is. Maybe U16GET inside of U16PUT is broken?
+	fprint(2, "clump at addr %d / %d\n", U16GET(buf+5), U16GET(buf+7));
+	tmp = U16GET(buf+5);
+	U16PUT(ci + 1, tmp);
+	U16PUT(ci + 3, U16GET(buf + 7));
+	fprint(2, "clump at addr %d / %d\n", U16GET(ci+1), U16GET(ci+3));
+
+Still showing 389, and then 65413 on readback. Why?
+
+...oh. For FUCKS SAKE. U16PUT doesn't work properly on char*. AHHHH.
+TODO: automated scan to make sure this mistake isn't made anywhere else. Or just add in the cast to the fucking macro.
+
+TODO: add inlining to kencc. If I wasn't using macros to avoid the perf hit of trivial functions, this would never have happened. Shouldn't have copied venti on this.
+
+Ohhh! It's U16GET that doesn't work, not put! The read in vtarenasync is still wrong!
+
+
+annnnd with that fixed in the cast, that venti instance is loading just fine, hahahahaha
--