change how memory is allocated to clean up a lot of messed up junk (mainly strange alignment adjustments which didnt completely make sense and frees which didnt match the allocation function that was used).

In case there are problems on other platforms (a few seems probable), here's the idea
1. Get rid of all manual alignment adjustments
2. use FCEU_malloc or malloc, as you see fit
3. use FCEU_free (or FCEU_gfree) if you use FCEU_malloc or FCEU_gmalloc. There's no real reason to need FCEU_gfree; this rule might be eliminated in the future (at which time FCEU_gfree will be removed)
4. If you need more alignment, increase it in FCEU_malloc. It's unlikely more alignment will ever be needed.

On windows, since the FCEU_*malloc functions now use aligned_alloc, we will catch instances where free() is used to free them (which happens frequently). allocates and frees should be matched. fix the free call if you ever observe this happening.

also

5. In general, remove crufty error handling for allocation failures. This just gunks up the code. If allocation fails, the application terminates.

7 files changed, 95 insertions, 65 deletions

diff --git a/src/drivers/common/vidblit.cpp b/src/drivers/common/vidblit.cpp
index 9b6f1c22..b2fbda24 100644
--- a/src/drivers/common/vidblit.cpp
+++ b/src/drivers/common/vidblit.cpp
@@ -238,18 +238,18 @@ void KillBlitToHigh(void)
 {

 	if(palettetranslate)

 	{

-		free(palettetranslate);

+		FCEU_free(palettetranslate);

 		palettetranslate=NULL;

 	}

 	

 	if(specbuf8bpp)

 	{

-		free(specbuf8bpp);

+		FCEU_free(specbuf8bpp);

 		specbuf8bpp = NULL;

 	}

 	if(specbuf32bpp)

 	{

-		free(specbuf32bpp);

+		FCEU_free(specbuf32bpp);

 		specbuf32bpp = NULL;

 	}

 	if(specbuf)

@@ -259,11 +259,11 @@ void KillBlitToHigh(void)
 			hq3x_Kill();

 		else

 			hq2x_Kill();

-		free(specbuf);

+		FCEU_free(specbuf);

 		specbuf=NULL;

 	}

 	if (nes_ntsc) {

-		free(nes_ntsc);

+		FCEU_free(nes_ntsc);

 		nes_ntsc = NULL;

 	}

 	if (ntscblit) {

diff --git a/src/ines.cpp b/src/ines.cpp
index c468df89..6766a558 100644
--- a/src/ines.cpp
+++ b/src/ines.cpp
@@ -108,11 +108,11 @@ void iNESGI(GI h) { //bbit edited: removed static keyword
 		if (iNESCart.Close)

 			iNESCart.Close();

 		if (ROM) {

-			free(ROM);

+			FCEU_free(ROM);

 			ROM = NULL;

 		}

 		if (VROM) {

-			free(VROM);

+			FCEU_free(VROM);

 			VROM = NULL;

 		}

 		if (trainerpoo) {

@@ -813,17 +813,11 @@ int iNESLoad(const char *name, FCEUFILE *fp, int OverwriteVidMode) {
 		}

 	}

 

-	if ((ROM = (uint8*)FCEU_malloc(ROM_size << 14)) == NULL)

-		return 0;

+	ROM = (uint8*)FCEU_malloc(ROM_size << 14);

 	memset(ROM, 0xFF, ROM_size << 14);

 

 	if (VROM_size) {

-		if ((VROM = (uint8*)FCEU_malloc(VROM_size << 13)) == NULL) {

-			free(ROM);

-			ROM = NULL;

-			FCEU_PrintError("Unable to allocate memory.");

-			return LOADER_HANDLED_ERROR;

-		}

+		VROM = (uint8*)FCEU_malloc(VROM_size << 13);

 		memset(VROM, 0xFF, VROM_size << 13);

 	}

 

diff --git a/src/state.cpp b/src/state.cpp
index a40e665e..78279c4b 100644
--- a/src/state.cpp
+++ b/src/state.cpp
@@ -307,13 +307,24 @@ static bool ReadStateChunks(EMUFILE* is, int32 totalsize)
 			// load back buffer

 			{

 				extern uint8 *XBackBuf;

-				if(is->fread((char*)XBackBuf,size) != size)

-					ret = false;

+				//ignore 8 garbage bytes, whose idea was it to write these or even have them there in the first place

+				if(size == 256*256+8)

+				{

+					if(is->fread((char*)XBackBuf,256*256) != 256*256)

+						ret = false;

+					is->fseek(8,SEEK_CUR);

+				}

+				else

+				{

+					if(is->fread((char*)XBackBuf,size) != size)

+						ret = false;

+				}

+

 

 				//MBG TODO - can this be moved to a better place?

 				//does it even make sense, displaying XBuf when its XBackBuf we just loaded?

 #ifdef __WIN_DRIVER__

-				else

+				if(ret)

 				{

 					FCEUD_BlitScreen(XBuf);

 					UpdateFCEUWindow();

@@ -404,7 +415,7 @@ bool FCEUSS_SaveMS(EMUFILE* outstream, int compressionLevel)
 	// save back buffer

 	{

 		extern uint8 *XBackBuf;

-		uint32 size = 256 * 256 + 8;

+		uint32 size = 256 * 256;

 		os->fputc(8);

 		write32le(size, os);

 		os->fwrite((char*)XBackBuf,size);

@@ -848,7 +859,7 @@ void ResetExState(void (*PreSave)(void), void (*PostSave)(void))
 	for(x=0;x<SFEXINDEX;x++)

 	{

 		if(SFMDATA[x].desc)

-			free( (void*)SFMDATA[x].desc);

+			FCEU_free( (void*)SFMDATA[x].desc);

 	}

 	// adelikat, 3/14/09:  had to add this to clear out the size parameter.  NROM(mapper 0) games were having savestate crashes if loaded after a non NROM game	because the size variable was carrying over and causing savestates to save too much data

 	SFMDATA[0].s = 0;

diff --git a/src/utils/memory.cpp b/src/utils/memory.cpp
index 80e072e1..e0eaf188 100644
--- a/src/utils/memory.cpp
+++ b/src/utils/memory.cpp
@@ -28,53 +28,70 @@
 #include "../fceu.h"
 #include "memory.h"
 
+static void *_FCEU_malloc(uint32 size)
+{
+	#ifdef _MSC_VER
+	void *ret = _aligned_malloc(size,32);
+	#else
+	void *ret = aligned_alloc(32,size);
+	#endif
+
+	if(!ret)
+	{
+		FCEU_PrintError("Error allocating memory!  Doing a hard exit.");
+		exit(1);
+	}
+
+	memset(ret, 0, size);
+
+	return ret;
+}
+
+static void _FCEU_free(void* ptr)
+{
+	#ifdef _MSC_VER
+	_aligned_free(ptr);
+	#else
+	aligned_free(ptr);
+	#endif
+}
+
 ///allocates the specified number of bytes. exits process if this fails
 void *FCEU_gmalloc(uint32 size)
 {
-	
- void *ret;
- ret=malloc(size);
- if(!ret)  
- {
-  FCEU_PrintError("Error allocating memory!  Doing a hard exit.");
-  exit(1);
- }
- FCEU_MemoryRand((uint8*)ret,size,true); // initialize according to RAMInitOption, default zero
+ void *ret = _FCEU_malloc(size);
+ 
+ // initialize according to RAMInitOption, default zero
+ FCEU_MemoryRand((uint8*)ret,size,true);
+
  return ret;
 }
 
-///allocates the specified number of bytes. returns null if this fails
 void *FCEU_malloc(uint32 size)
 {
- void *ret;
- ret=malloc(size);
- if(!ret)
- {
-  FCEU_PrintError("Error allocating memory!");
-  return(0);
- }
- memset(ret,0,size); // initialize to 0
- return ret;
+	void *ret = _FCEU_malloc(size);
+	memset(ret, 0, size);
+	return ret;
 }
 
 ///frees memory allocated with FCEU_gmalloc
 void FCEU_gfree(void *ptr)
 {
- free(ptr);
+	_FCEU_free(ptr);
 }
 
 ///frees memory allocated with FCEU_malloc
-void FCEU_free(void *ptr)    // Might do something with this and FCEU_malloc later...
+void FCEU_free(void *ptr)
 {
- free(ptr);
+	_FCEU_free(ptr);
 }
 
 void *FCEU_dmalloc(uint32 size)
 {
-    return malloc(size);
+	return FCEU_malloc(size);
 }
 
 void FCEU_dfree(void *ptr)
 {
-    free(ptr);
+	return FCEU_free(ptr);
 }
diff --git a/src/utils/memory.h b/src/utils/memory.h
index 559c7bad..174b3d31 100644
--- a/src/utils/memory.h
+++ b/src/utils/memory.h
@@ -24,13 +24,21 @@
 
 #define FCEU_dwmemset(d,c,n) {int _x; for(_x=n-4;_x>=0;_x-=4) *(uint32 *)&(d)[_x]=c;}
 
-void *FCEU_malloc(uint32 size); // initialized to 0
-void *FCEU_gmalloc(uint32 size); // used by boards for WRAM etc, initialized to 0 (default) or other via RAMInitOption
+//returns a 32-aligned buffer, initialized to 0
+void *FCEU_malloc(uint32 size); 
+
+//returns a 32-aligned buffer, with jumbled initial contents
+//used by boards for WRAM etc, initialized to 0 (default) or other via RAMInitOption
+void *FCEU_gmalloc(uint32 size); 
+
+//free memory allocated with FCEU_gmalloc
 void FCEU_gfree(void *ptr);
+
+//free memory allocated with 
 void FCEU_free(void *ptr);
-void FCEU_memmove(void *d, void *s, uint32 l);
 
-// wrapper for debugging when its needed, otherwise act like
-// normal malloc/free
+//don't use these. change them if you find them.
 void *FCEU_dmalloc(uint32 size);
+
+//don't use these. change them if you find them.
 void FCEU_dfree(void *ptr);
diff --git a/src/video.cpp b/src/video.cpp
index 531c5f3c..cbad58fc 100644
--- a/src/video.cpp
+++ b/src/video.cpp
@@ -116,27 +116,17 @@ void FCEU_KillVirtualVideo(void)
 int FCEU_InitVirtualVideo(void)

 {

 	//Some driver code may allocate XBuf externally.

-	//256 bytes per scanline, * 240 scanline maximum, +16 for alignment,

+	//256 bytes per scanline, * 240 scanline maximum

 	if(XBuf)

 		return 1;

 	

-	XBuf = (u8*)FCEU_malloc(256 * 256 + 16);

-	XBackBuf = (u8*)FCEU_malloc(256 * 256 + 16);

-	XDBuf = (u8*)FCEU_malloc(256 * 256 + 16);

-	XDBackBuf = (u8*)FCEU_malloc(256 * 256 + 16);

-	if(!XBuf || !XBackBuf || !XDBuf || !XDBackBuf)

-	{

-		return 0;

-	}

+	XBuf = (u8*)FCEU_malloc(256 * 256);

+	XBackBuf = (u8*)FCEU_malloc(256 * 256);

+	XDBuf = (u8*)FCEU_malloc(256 * 256);

+	XDBackBuf = (u8*)FCEU_malloc(256 * 256);

 

-	xbsave = XBuf;

 

-	if( sizeof(uint8*) == 4 )

-	{

-		uintptr_t m = (uintptr_t)XBuf;

-		m = ( 8 - m) & 7;

-		XBuf+=m;

-	}

+	xbsave = XBuf;

 

 	memset(XBuf,128,256*256);

 	memset(XBackBuf,128,256*256);

diff --git a/src/video.h b/src/video.h
index 64e0688b..7fbb6bad 100644
--- a/src/video.h
+++ b/src/video.h
@@ -7,10 +7,20 @@ int SaveSnapshot(char[]);
 void ResetScreenshotsCounter();

 uint32 GetScreenPixel(int x, int y, bool usebackup);

 int GetScreenPixelPalette(int x, int y, bool usebackup);

+

+//in case we need more flags in the future we can change the size here

+//bit0 : monochrome bit

+//bit5 : emph red

+//bit6 : emph green

+//bit7 : emph blue

+typedef uint8 xfbuf_t;

+

 extern uint8 *XBuf;

 extern uint8 *XBackBuf;

 extern uint8 *XDBuf;

 extern uint8 *XDBackBuf;

+extern xfbuf_t *XFBuf;

+

 extern int ClipSidesOffset;

 

 struct GUIMESSAGE