- Fixed: If you called the FString assignment operator that accepts a

const char * with a string inside its buffer, it released the buffer
  before copying the string.
- Added a new FString constructor that creates the string from a lump.
- Fixed: G_DoReborn() calls G_InitNew() with mapname set to level.mapname.
  G_InitNew() then copies it onto level.mapname, which is undefined
  behavior (although it does work as we want it to).
- Modified FMemLump to store its data using FString. That class provides
  a convenient method of storing reference counted data, so now FMemLump
  doesn't need to muck about sneakily using const_casts and possibly
  tricking its users into thinking that an old one is still valid after
  being assigned to a new one.
- Fixed: The IMGZ, PNG, PCX, and JPEG loaders assumed the files were
  large enough for their headers without actually checking.



SVN r463 (trunk)

Makefile.linux

@@ -51,7 +51,10 @@ ifdef DEBUG
 else
   OBJDIR = $(RELEASEOBJ)
   CFLAGS += -DNDEBUG
-  LDFLAGS += -s -Wl,-Map=$(ZDOOM).map
+  LDFLAGS += -Wl,-Map=$(ZDOOM).map
+  ifndef NOSTRIP
+    LDFLAGS += -s
+  endif
   ZDOOMBIN = $(ZDOOM)
 endif
 CXXFLAGS += $(CFLAGS)
@@ -139,4 +142,4 @@ ccdv: ccdv-posix.c
 
 ifeq (,$(findstring $(MAKECMDGOALS),clean cleandeps cleanobjs distclean toolsandpk3 cleantools updaterev))
 -include $(DEPS)
-endif
+endif

docs/rh-log.txt

@@ -1,3 +1,21 @@
+January 24, 2007
+- Fixed: If you called the FString assignment operator that accepts a
+  const char * with a string inside its buffer, it released the buffer
+  before copying the string.
+- Added a new FString constructor that creates the string from a lump.
+
+January 23, 2007
+- Fixed: G_DoReborn() calls G_InitNew() with mapname set to level.mapname.
+  G_InitNew() then copies it onto level.mapname, which is undefined
+  behavior (although it does work as we want it to).
+- Modified FMemLump to store its data using FString. That class provides
+  a convenient method of storing reference counted data, so now FMemLump
+  doesn't need to muck about sneakily using const_casts and possibly
+  tricking its users into thinking that an old one is still valid after
+  being assigned to a new one.
+- Fixed: The IMGZ, PNG, PCX, and JPEG loaders assumed the files were
+  large enough for their headers without actually checking.
+
 January 22, 2007
 - Fixed: The simulated palette blends when the menu or console are open
   were applied even when the status bar wasn't drawn. (In other words, even

src/g_level.cpp

@@ -1580,7 +1580,10 @@ void G_InitNew (const char *mapname, bool bTitleLevel)
 		bglobal.Init ();
 	}
 
-	strcpy (level.mapname, mapname);
+	if (mapname != level.mapname)
+	{
+		strcpy (level.mapname, mapname);
+	}
 	if (bTitleLevel)
 	{
 		gamestate = GS_TITLELEVEL;

src/sdl/i_main.cpp

@@ -177,6 +177,8 @@ static int DoomSpecificInfo (char *buffer, char *end)
 	return p;
 }
 
+#include "zstring.h"
+
 int main (int argc, char **argv)
 {
 	printf("ZDoom v%s - SVN revision %s - SDL version\nCompiled on %s\n\n",

src/textures/imgztexture.cpp

@@ -57,8 +57,7 @@ bool FIMGZTexture::Check(FileReader & file)
 {
 	DWORD id;
 	file.Seek(0, SEEK_SET);
-	file.Read(&id, 4);
-	return (id == MAKE_ID('I','M','G','Z'));
+	return file.Read(&id, 4) == 4 && id == MAKE_ID('I','M','G','Z');
 }
 
 FTexture *FIMGZTexture::Create(FileReader & file, int lumpnum)

src/textures/jpegtexture.cpp

@@ -100,8 +100,7 @@ bool FJPEGTexture::Check(FileReader & file)
 {
 	BYTE hdr[3];
 	file.Seek(0, SEEK_SET);
-	file.Read(hdr, 3);
-	return (hdr[0] == 0xFF && hdr[1] == 0xD8 && hdr[2] == 0xFF);
+	return file.Read(hdr, 3) == 3 && hdr[0] == 0xFF && hdr[1] == 0xD8 && hdr[2] == 0xFF;
 }
 
 FTexture *FJPEGTexture::Create(FileReader & data, int lumpnum)

src/textures/pcxtexture.cpp

@@ -46,7 +46,10 @@ bool FPCXTexture::Check(FileReader & file)
 	PCXHeader hdr;
 
 	file.Seek(0, SEEK_SET);
-	file.Read(&hdr, sizeof(hdr));
+	if (file.Read(&hdr, sizeof(hdr)) != sizeof(hdr))
+	{
+		return false;
+	}
 
 #ifdef WORDS_BIGENDIAN
 	hdr.xmin = LittleShort(hdr.xmin);

src/textures/pngtexture.cpp

@@ -45,8 +45,7 @@ bool FPNGTexture::Check(FileReader & file)
 {
 	DWORD id;
 	file.Seek(0, SEEK_SET);
-	file.Read(&id, 4);
-	return (id == MAKE_ID(137,'P','N','G'));
+	return file.Read(&id, 4) == 4 && id == MAKE_ID(137,'P','N','G');
 }
 
 FTexture *FPNGTexture::Create(FileReader & data, int lumpnum)

src/thingdef_codeptr.cpp

@@ -2064,7 +2064,7 @@ void A_SetGravity(AActor * self)
 	int index=CheckIndex(1);
 	if (index<0) return;
 	
-	self->gravity = clamp<fixed_t> (EvalExpressionF (StateParameters[index], self), 0, FRACUNIT); 
+	self->gravity = clamp<fixed_t> (fixed_t(EvalExpressionF (StateParameters[index], self)*FRACUNIT), 0, FRACUNIT); 
 }
 
 

src/w_wad.cpp

@@ -1637,17 +1637,7 @@ void FWadCollection::ReadLump (int lump, void *dest)
 
 FMemLump FWadCollection::ReadLump (int lump)
 {
-	FWadLump lumpr = OpenLumpNum (lump);
-	long size = lumpr.GetLength ();
-	BYTE *dest = new BYTE[size];
-	long numread = lumpr.Read (dest, size);
-
-	if (numread != size)
-	{
-		I_Error ("W_ReadLump: only read %ld of %ld on lump %i\n",
-			numread, size, lump);	
-	}
-	return FMemLump (dest);
+	return FMemLump(FString(ELumpNum(lump)));
 }
 
 //==========================================================================
@@ -2230,45 +2220,41 @@ long FWadLump::Read (void *buffer, long len)
 // FMemLump -----------------------------------------------------------------
 
 FMemLump::FMemLump ()
-: Block (NULL)
 {
 }
 
-#ifdef __GNUC__
 FMemLump::FMemLump (const FMemLump &copy)
-#else
-FMemLump::FMemLump (FMemLump &copy)
-#endif
 {
 	Block = copy.Block;
-	const_cast<FMemLump *>(&copy)->Block = NULL;
 }
 
-#ifdef __GNUC__
 FMemLump &FMemLump::operator = (const FMemLump &copy)
-#else
-FMemLump &FMemLump::operator = (FMemLump &copy)
-#endif
 {
-	if (Block != NULL)
-	{
-		delete[] Block;
-	}
 	Block = copy.Block;
-	const_cast<FMemLump *>(&copy)->Block = NULL;
 	return *this;
 }
 
-FMemLump::FMemLump (BYTE *data)
-: Block (data)
+FMemLump::FMemLump (const FString &source)
+: Block (source)
 {
 }
 
 FMemLump::~FMemLump ()
 {
-	if (Block != NULL)
+}
+
+FString::FString (ELumpNum lumpnum)
+{
+	FWadLump lumpr = Wads.OpenLumpNum ((int)lumpnum);
+	long size = lumpr.GetLength ();
+	AllocBuffer (1 + size);
+	long numread = lumpr.Read (&Chars[0], size);
+	Chars[size] = '\0';
+
+	if (numread != size)
 	{
-		delete[] Block;
+		I_Error ("ConstructStringFromLump: Only read %ld of %ld bytes on lump %i (%s)\n",
+			numread, size, lumpnum, Wads.GetLumpFullName((int)lumpnum));
 	}
 }
 

src/w_wad.h

@@ -123,29 +123,21 @@ private:
 };
 
 
-// A lump in memory. The destructor automatically deletes the memory
-// the lump was copied to. Note the copy contstructor is really more of
-// a transfer constructor. Once an FMemLump gets copied, the original
-// is no longer usable.
+// A lump in memory.
 class FMemLump
 {
 public:
 	FMemLump ();
-#ifdef __GNUC__
-	// Not really const! GCC forces me to declare it this way!
+
 	FMemLump (const FMemLump &copy);
 	FMemLump &operator= (const FMemLump &copy);
-#else
-	FMemLump (FMemLump &copy);
-	FMemLump &operator= (FMemLump &copy);
-#endif
 	~FMemLump ();
-	void *GetMem () { return (void *)Block; }
+	void *GetMem () { return (void *)Block.GetChars(); }
 
 private:
-	FMemLump (BYTE *data);
+	FMemLump (const FString &source);
 
-	BYTE *Block;
+	FString Block;
 
 	friend class FWadCollection;
 };

src/zstring.cpp

@@ -172,17 +172,34 @@ FString &FString::operator = (const FString &other)
 }
 FString &FString::operator = (const char *copyStr)
 {
-	Data()->Release();
-	if (copyStr == NULL || *copyStr == '\0')
+	if (copyStr != Chars)
 	{
-		NullString.RefCount++;
-		Chars = &NullString.Nothing[0];
-	}
-	else
-	{
-		size_t len = strlen (copyStr);
-		AllocBuffer (len);
-		StrCopy (Chars, copyStr, len);
+		if (copyStr == NULL || *copyStr == '\0')
+		{
+			NullString.RefCount++;
+			Chars = &NullString.Nothing[0];
+		}
+		else
+		{
+			// In case copyStr is inside us, we can't release it until
+			// we've finished the copy.
+			FStringData *old = Data();
+
+			if (copyStr < Chars || copyStr >= Chars + old->Len)
+			{
+				// We know the string isn't in our buffer, so release it now
+				// to reduce the potential for needless memory fragmentation.
+				old->Release();
+				old = NULL;
+			}
+			size_t len = strlen (copyStr);
+			AllocBuffer (len);
+			StrCopy (Chars, copyStr, len);
+			if (old != NULL)
+			{
+				old->Release();
+			}
+		}
 	}
 	return *this;
 }

src/zstring.h

@@ -68,6 +68,10 @@ struct FNullStringData
 	char Nothing[2];
 };
 
+enum ELumpNum
+{
+};
+
 class FString
 {
 public:
@@ -87,6 +91,9 @@ public:
 	FString (const char *head, const char *tail);
 	FString (char head, const FString &tail);
 
+	// Other constructors
+	FString (ELumpNum);	// Create from a lump
+
 	~FString ();
 
 	char *LockBuffer();		// Obtain write access to the character buffer