fix some embarrassing bugs in new snes interop. maybe biz will be more stable now.

BizHawk.Emulation.Cores/Consoles/Nintendo/SNES/LibsnesApi.cs

@@ -27,8 +27,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 
 		CommStruct* comm;
 		MessageApi Message;
-		BufferApi CopyBuffer; //TODO: consider making private and wrapping
+		BufferApi _copyBuffer; //TODO: consider making private and wrapping
-		BufferApi SetBuffer; //TODO: consider making private and wrapping
+		BufferApi _setBuffer; //TODO: consider making private and wrapping
 
 		public LibsnesApi(string dllPath)
 		{
@@ -36,8 +36,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 			instanceDll = new InstanceDll(dllPath);
 			var dllinit = (DllInit)Marshal.GetDelegateForFunctionPointer(instanceDll.GetProcAddress("DllInit"), typeof(DllInit));
 			Message = (MessageApi)Marshal.GetDelegateForFunctionPointer(instanceDll.GetProcAddress("Message"), typeof(MessageApi));
-			CopyBuffer = (BufferApi)Marshal.GetDelegateForFunctionPointer(instanceDll.GetProcAddress("CopyBuffer"), typeof(BufferApi));
+			_copyBuffer = (BufferApi)Marshal.GetDelegateForFunctionPointer(instanceDll.GetProcAddress("CopyBuffer"), typeof(BufferApi));
-			SetBuffer = (BufferApi)Marshal.GetDelegateForFunctionPointer(instanceDll.GetProcAddress("SetBuffer"), typeof(BufferApi));
+			_setBuffer = (BufferApi)Marshal.GetDelegateForFunctionPointer(instanceDll.GetProcAddress("SetBuffer"), typeof(BufferApi));
 
 			comm = (CommStruct*)dllinit().ToPointer();
 		}
@@ -52,33 +52,50 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 			DeallocatedMemoryBlocks.Clear();
 		}
 
-		public void CopyString(string str)
+		/// <summary>
+		/// Copy an ascii string into libretro. It keeps the copy.
+		/// </summary>
+		public void CopyAscii(int id, string str)
 		{
-			fixed (char* cp = str)
+			fixed (byte* cp = System.Text.Encoding.ASCII.GetBytes(str+"\0"))
-				CopyBuffer(0, cp, str.Length + 1);
+				_copyBuffer(id, cp, str.Length + 1);
 		}
 
-		public void CopyBytes(byte[] bytes)
+		/// <summary>
+		/// Copy a buffer into libretro. It keeps the copy.
+		/// </summary>
+		public void CopyBytes(int id, byte[] bytes)
 		{
 			fixed (byte* bp = bytes)
-				CopyBuffer(0, bp, bytes.Length);
+				_copyBuffer(id, bp, bytes.Length);
 		}
 
-		public void SetAscii(string str)
+		/// <summary>
+		/// Locks a buffer and sets it into libretro. You must pass a delegate to be executed while that buffer is locked.
+		/// This is meant to be used for avoiding a memcpy for large roms (which the core is then just going to memcpy again on its own)
+		/// The memcpy has to happen at some point (libretro semantics specify [not literally, the docs dont say] that the core should finish using the buffer before its init returns)
+		/// but this limits it to once.
+		/// Moreover, this keeps the c++ side from having to free strings when they're no longer used (and memory management is trickier there, so we try to avoid it)
+		/// </summary>
+		public void SetBytes(int id, byte[] bytes, Action andThen)
 		{
-			fixed (char* cp = str)
+			fixed (byte* bp = bytes)
-				SetBuffer(0, cp, str.Length + 1);
+			{
+				_setBuffer(id, bp, bytes.Length);
+				andThen();
+			}
 		}
 
-		public void SetBytes(byte[] bytes)
+		/// <summary>
+		/// see SetBytes
+		/// </summary>
+		public void SetAscii(int id, string str, Action andThen)
 		{
-			fixed (byte* bp = bytes)
+			fixed (byte* cp = System.Text.Encoding.ASCII.GetBytes(str+"\0"))
-				SetBuffer(0, bp, bytes.Length);
+			{
-		}
+				_setBuffer(id, cp, str.Length + 1);
-		public void SetBytes2(byte[] bytes)
+				andThen();
-		{
+			}
-			fixed (byte* bp = bytes)
-				SetBuffer(1, bp, bytes.Length);
 		}
 
 		public Action<uint> ReadHook, ExecHook;
@@ -127,7 +144,6 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 		public delegate void snes_trace_t(string msg);
 
 
-		[StructLayout(LayoutKind.Sequential)]
 		public struct CPURegs
 		{
 			public uint pc;
@@ -137,7 +153,6 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 			public byte sp, dp, db, mdr;
 		}
 
-		[StructLayout(LayoutKind.Sequential)]
 		public struct LayerEnables
 		{
 			byte _BG1_Prio0, _BG1_Prio1;
@@ -161,7 +176,6 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 			public bool Obj_Prio3 { get { return _Obj_Prio3 != 0; } set { _Obj_Prio3 = (byte)(value ? 1 : 0); } }
 		}
 
-		[StructLayout(LayoutKind.Sequential)]
 		struct CommStruct
 		{
 			//the cmd being executed
@@ -185,8 +199,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 			public fixed int inports[2];
 
 			//this should always be used in pairs
-			public void* buf0, buf1;
+			public fixed uint buf[3]; //ACTUALLY A POINTER but can't marshal it :(
-			public int buf_size0, buf_size1;
+			public fixed int buf_size[3];
 
 			//bleck. this is a long so that it can be a 32/64bit pointer
 			public fixed long cdl_ptr[4];

BizHawk.Emulation.Cores/Consoles/Nintendo/SNES/LibsnesApi_CMD.cs

@@ -8,8 +8,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 	{
 		public bool CMD_serialize(IntPtr data, int size)
 		{
-			comm->buf0 = data.ToPointer();
+			comm->buf[0] = (uint)data.ToInt32();
-			comm->buf_size0 = size;
+			comm->buf_size[0] = size;
 			Message(eMessage.eMessage_CMD_serialize);
 			WaitForCMD();
 			bool ret = comm->GetBool();
@@ -29,8 +29,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 
 		public bool CMD_unserialize(IntPtr data, int size)
 		{
-			comm->buf0 = data.ToPointer();
+			comm->buf[0] = (uint)data.ToInt32();
-			comm->buf_size0 = size;
+			comm->buf_size[0] = size;
 			Message(eMessage.eMessage_CMD_unserialize);
 			WaitForCMD();
 			bool ret = comm->GetBool();
@@ -61,28 +61,28 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 
 		public bool CMD_load_cartridge_super_game_boy(string rom_xml, byte[] rom_data, uint rom_size, byte[] dmg_data)
 		{
-			fixed (char* xmlcp = rom_xml)
+			SetAscii(0, rom_xml, () =>
-			{
+				 SetBytes(1, rom_data, () =>
-				comm->str = (sbyte*)xmlcp;
+					 SetBytes(2, dmg_data, () =>
-				SetBytes(rom_data);
+					 {
-				SetBytes2(dmg_data);
+						 Message(eMessage.eMessage_CMD_load_cartridge_sgb);
-				Message(eMessage.eMessage_CMD_load_cartridge_sgb);
+						 WaitForCMD();
-				WaitForCMD();
+					 })
-				return comm->GetBool();
+				 )
-			}
+			);
+			return comm->GetBool();
 		}
 
 		public bool CMD_load_cartridge_normal(byte[] rom_xml, byte[] rom_data)
 		{
 			string xml = rom_xml==null?null:System.Text.Encoding.ASCII.GetString(rom_xml);
-			fixed (char* xmlcp = xml)
+			SetAscii(0, xml??"", () =>
-			{
+				SetBytes(1, rom_data, () => {
-				comm->str = (sbyte*)xmlcp;
+					Message(eMessage.eMessage_CMD_load_cartridge_normal);
-				SetBytes(rom_data);
+					WaitForCMD();
-				Message(eMessage.eMessage_CMD_load_cartridge_normal);
+				})
-				WaitForCMD();
+			);
-				return comm->GetBool();
+			return comm->GetBool();
-			}
 		}
 
 		public void CMD_term()

BizHawk.Emulation.Cores/Consoles/Nintendo/SNES/LibsnesApi_SIG.cs

@@ -65,7 +65,7 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 						string ret = hint;
 						if (pathRequest != null)
 						  hint = pathRequest(slot, hint);
-						SetAscii(hint);
+						CopyAscii(0, hint);
 						break;
 					}
 				case eMessage.eMessage_SIG_trace_callback:
@@ -109,7 +109,7 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
 
 						comm->ptr = smb.Ptr;
 						SharedMemoryBlocks[smb.Name] = smb;
-						CopyString(smb.BlockName);
+						CopyAscii(0, smb.BlockName);
 						break;
 					}
 				case eMessage.eMessage_SIG_freeSharedMemory:

libsnes/bsnes/target-libsnes/libsnes_pwrap.cpp

@@ -150,8 +150,8 @@ struct CommStruct
 	SNES::Input::Device inports[2];
 
 	//always used in pairs
-	void* buf[2];
+	void* buf[3];
-	int32 buf_size[2];
+	int32 buf_size[3];
 
 	int64 cdl_ptr[4];
 	int32 cdl_size[4];
@@ -167,11 +167,7 @@ struct CommStruct
 
 	//private stuff
 	void* privbuf[2]; //TODO remember to tidy this..
-	void SetString(const char* str)
+
-	{
-		if (privbuf[0]) free(privbuf[0]);
-		buf[0] = privbuf[0] = strdup(str);
-	}
 	void CopyBuffer(int id, void* ptr, int32 size)
 	{
 		if (privbuf[id]) free(privbuf[id]);
@@ -182,13 +178,12 @@ struct CommStruct
 
 	void SetBuffer(int id, void* ptr, int32 size)
 	{
+		if (privbuf[id]) free(privbuf[id]);
+		privbuf[id] = nullptr;
 		buf[id] = ptr;
 		buf_size[id] = size;
 	}
 
-	struct {
-	} strings;
-
 
 } comm;
 
@@ -202,7 +197,8 @@ uint16_t audiobuffer[AUDIOBUFFER_SIZE];
 int audiobuffer_idx = 0;
 Action CMD_cb;
 
-void BREAK(eMessage msg) {
+void BREAK(eMessage msg)
+{
 	comm.status = eStatus_BRK;
 	comm.reason = msg;
 	co_emu_suspended = co_active();
@@ -273,7 +269,7 @@ const char* snes_path_request(int slot, const char* hint)
 	comm.slot = slot;
 	comm.str= (char *)hint;
 	BREAK(eMessage_SIG_path_request);
-	return (const char*)comm.buf;
+	return (const char*)comm.buf[0];
 }
 
 void snes_scanlineStart(int line)
@@ -358,7 +354,9 @@ static void Analyze()
 
 void CMD_LoadCartridgeNormal()
 {
-	bool ret = snes_load_cartridge_normal(comm.str, (const uint8_t*)comm.buf[0], comm.buf_size[0]);
+	const char* xml = (const char*)comm.buf[0];
+	if(!xml[0]) xml = nullptr;
+	bool ret = snes_load_cartridge_normal(xml, (const uint8_t*)comm.buf[1], comm.buf_size[1]);
 	comm.value = ret?1:0;
 
 	if(ret)
@@ -367,7 +365,7 @@ void CMD_LoadCartridgeNormal()
 
 void CMD_LoadCartridgeSGB()
 {
-	bool ret = snes_load_cartridge_super_game_boy(comm.str, (const u8*)comm.buf[0], comm.buf_size[0], nullptr, (const u8*)comm.buf[1], comm.buf_size[1]);
+	bool ret = snes_load_cartridge_super_game_boy((const char*)comm.buf[0], (const u8*)comm.buf[1], comm.buf_size[1], nullptr, (const u8*)comm.buf[2], comm.buf_size[2]);
 	comm.value = ret ? 1 : 0;
 
 	if(ret)
@@ -382,7 +380,7 @@ void CMD_init()
 	SNES::input.connect(SNES::Controller::Port2, comm.inports[1]);
 }
 
-void CMD_serialize()
+void CMD_Serialize()
 {
 	int size = comm.buf_size[0];
 	char* buf = (char*)comm.buf[0];
@@ -390,7 +388,7 @@ void CMD_serialize()
 	comm.value = ret ? 1 : 0;
 }
 
-void CMD_unserialize()
+void CMD_Unserialize()
 {
 	int size = comm.buf_size[0];
 	char* buf = (char*)comm.buf[0];
@@ -398,7 +396,7 @@ void CMD_unserialize()
 	comm.value = ret ? 1 : 0;
 }
 
-static void snes_run()
+static void CMD_Run()
 {
 	do_SIG_audio_flush();
 
@@ -523,9 +521,9 @@ const Action kHandlers_CMD[] = {
 	CMD_init,
 	snes_power,
 	snes_reset,
-	snes_run,
+	CMD_Run,
-	CMD_serialize,
+	CMD_Serialize,
-	CMD_unserialize,
+	CMD_Unserialize,
 	CMD_LoadCartridgeNormal,
 	CMD_LoadCartridgeSGB,
 	snes_term,
@@ -580,6 +578,8 @@ BOOL WINAPI DllMain(_In_ HINSTANCE hinstDLL, _In_ DWORD     fdwReason, _In_ LPVO
 
 extern "C" dllexport void* __cdecl DllInit()
 {
+	memset(&comm,0,sizeof(comm));
+
 	//make a coroutine thread to run the emulation in. we'll switch back to this cothread when communicating with the frontend
 	co_control = co_active();
 	co_emu = co_create(65536 * sizeof(void*), new_emuthread);