Sign in to follow this  

Logging Class

This topic is 3728 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi. I'm currently trying to make a 2d platform game using C++. I'm using Turbo C++ 3.0 (Being forced to more like it). Anyway I was getting a lot of memory leaks. So I decided to implement a class for Logging (Can't use external Libraries). The simplest way I could find was to overload the new and delete operator to include the current LINE NO and FILE. And then to use a macro to replace new with the "Newer" version of new. Unfortunately Turbo C++ does not allow you to overload the global new/delete operator for more than one argument. So I'm stuck with size_t as the parameters. Another way I could think of was to allocate some extra amount of memory in which I found store some kind of special symbol (Sentinel) and the size of the object being allocated. But for some reason when I would try to read the data in delete I could never find the Sentinel. (ie Data got corrupted) The code was something like this -
void *operator new(size_t size)
{
 void *p;
 size_t newSize = size + strlen(Sentinel) + sizeof(size_t);
 
 if((p = malloc(size)) == 0)
 {
  return 0;
 }
 
 char *s = (char*)p;
 strcpy(s+size, Sentinel);
 //size_t is of size 2 bytes.
 s[newSize - 2] = (size >> 8); //highbyte
 s[newSize - 1] = (size); //lowbyte
 
 p = (void*)s;
 //Log the amount of memory used.
 return p;
}
//Delete operator similar except it searches for the Sentinel and then 
//gets the size (which does not work)

After trying all sorts of variations of the above Code I finally chucked this method and tried an alternate method where I have a struct MemoryEntry with contains a void *, size, valid. There is an array of about 150 of these MemoryEntry's (Yes I know I should use a linked List, or even better a hash table, but I currently don't feel like Implementing one myself right now.). In which I store the data. I'll just post the code. It's a little haphazard but I hope you can understand it. (The Logger Class is a singleton Class) Header File
struct MemoryEntry {
	void * memory;
	size_t size;
	int valid;

	MemoryEntry()
	: memory(0), size(0), valid(0)
	{}
};

class Logger {
	ofstream logFile;
	long memoryUsed;
	int index;
	
	MemoryEntry memory[150];
	
	Logger(const char * fileName);
	void LogHeader();
	void LogMemoryStatus();
public :
	~Logger();
	static Logger* GetInstance();

	void AddMemoryEntry(MemoryEntry & e);
	void RemoveMemoryEntry(void * p);
	
	void LogTime();
	void LogConstructor();
	void LogDestructor();
	void LogFunction();
	void LogError();
	
	Logger & operator<<(const char * a);
	Logger & operator<<(char a);
	Logger & operator<<(int a);
	Logger & operator<<(float a);
	Logger & operator<<(long a);
	Logger & operator<<(double a);
	Logger & operator<<(unsigned char a);
	Logger & operator<<(unsigned int a);
	Logger & operator<<(unsigned long a);
	Logger & operator<<(void * a);
};

And here is some part of the Implementation
#ifdef _MEMORY_DEBUG_
void *operator new(size_t size)
{
	void *p;

	Logger * Log = Logger::GetInstance();
	if((p = malloc(size)) == 0)
	{
		Log->LogError();
		*Log << "Memory Allocation Failure\n";
		exit(1);
	}

	MemoryEntry e;
	e.memory = p;
	e.size 	= size;
	e.valid 	= true;

	Log->AddMemoryEntry(e);
	return p;
}

void operator delete(void *p)
{
	if(p == 0)
		return;

	Logger * Log = Logger::GetInstance();
	Log->RemoveMemoryEntry(p);
	
	if(p)
		free(p);
	p = 0;
}
#endif

Logger * Logger::GetInstance()
{
	static Logger log(FILENAME);
	return &log;
}

Logger::Logger(const char * fileName)
{
	#ifdef _DEBUG_
	logFile.open(fileName, ios::app);
	#endif
	memoryUsed = 0;
	index = 0;
	LogHeader();
}

Logger::~Logger()
{
	LogMemoryStatus();
	
	#ifdef _MEMORY_DEBUG_
	logFile << "\nIndex - Size - Valid\n";
	for(int i = 0; i<index; i++)
	{
		logFile	<< setw(5) << i << "  " << setw(5)
					<<memory[i].size << " - "
					<< memory[i].valid << "\n";
	}
	
	int numLeft=0;
	
	logFile << "\nCleaning up Memory\n";
	for(i = 0; i<index; i++)
		if(memory[i].valid == 1)
		{
			numLeft++;
			memoryUsed -= memory[i].size;
			LogTime();
			logFile 	<< "--- - " 
					<< setw(5) << memory[i].size 
					<< " bytes DeAllocated - "
					<< setw(6) << memoryUsed	
					<< " - " << setw(2) << i << "\n";
						
			memory[i].valid = 0;
		}
	#endif
	logFile << "---------------------------\n\n\n\n";
	logFile.close();
}
void Logger::AddMemoryEntry(MemoryEntry & e)
{
	memory[index] = e;
	memoryUsed += e.size;
	
	LogTime();
	logFile << "--- - " << setw(5) << e.size 
		<< " bytes   Allocated - " << setw(6) << memoryUsed
		<< " - " << setw(2) << index << "\n";
	
	index++;
}

void Logger::RemoveMemoryEntry(void *p)
{
	for(int i=0; i< index; i++)
	{
		if(memory[i].memory == p)
		{
			
			memoryUsed -= memory[i].size;
			LogTime();
			logFile << "--- - " << setw(5) << memory[i].size 
				<< " bytes DeAllocated - " 
				<< setw(6) << memoryUsed << " - " 
				<< setw(2) << i << "\n";
			memory[i].valid = 0;
			return;
		}
	}
	LogError();
	logFile << "Could not find memory address in table - Deleteing Anyway\n";
}

But this Method too does not work very well as It's quite difficult finding exactly where the memory leak is occuring. And quite often some how the pointer recieved in delete is not present in the memory array. (Don't know why). If someone could suggest an even better way of Logging I would be absolutely Thrilled. I'm Posting a short portion of the LogFile so you can see how it looks
10/04/07 ---- 03:19:31 ----
03:19:31 : --- -    56 bytes   Allocated -     56 -  0
03:19:31 : --- -   516 bytes   Allocated -    572 -  1
03:19:31 : --- -    56 bytes   Allocated -    628 -  2
03:19:31 : --- -   516 bytes   Allocated -   1144 -  3
03:19:31 : --- -    56 bytes   Allocated -   1200 -  4
03:19:31 : --- -   516 bytes   Allocated -   1716 -  5
03:19:31 : -C- -           VGA
03:19:31 : --- - 64000 bytes   Allocated -  65716 -  6
03:19:31 : --- - 64000 bytes   Allocated - 129716 -  7
03:19:31 : -C- -           Bitmap - \B1.bmp
03:19:31 : -F- -           Bitmap Open
03:19:31 : --- -   516 bytes   Allocated - 130232 -  8
03:19:31 : --- - 64000 bytes   Allocated - 194232 -  9
03:19:31 : --- -   516 bytes DeAllocated - 193716 -  8
03:19:31 : -D- -           Bitmap
03:19:31 : --- - 64000 bytes DeAllocated - 129716 -  9
03:19:31 : -D- -           VGA
03:19:31 : -E- - Could not find memory address in table - Deleteing Anyway
03:19:31 : -E- - Could not find memory address in table - Deleteing Anyway

129716 B of Memory Allocated from the Heap Left.

Index - Size - Valid
    0     56 - 1
    1    516 - 1
    2     56 - 1
    3    516 - 1
    4     56 - 1
    5    516 - 1
    6  64000 - 1
    7  64000 - 1
    8    516 - 0
    9  64000 - 0

Cleaning up Memory
03:19:31 : --- -    56 bytes DeAllocated - 129660 -  0
03:19:31 : --- -   516 bytes DeAllocated - 129144 -  1
03:19:31 : --- -    56 bytes DeAllocated - 129088 -  2
03:19:31 : --- -   516 bytes DeAllocated - 128572 -  3
03:19:31 : --- -    56 bytes DeAllocated - 128516 -  4
03:19:31 : --- -   516 bytes DeAllocated - 128000 -  5
03:19:32 : --- - 64000 bytes DeAllocated -  64000 -  6
03:19:32 : --- - 64000 bytes DeAllocated -      0 -  7
---------------------------


I'm someone could help me with this I would really appreciate it.

Share this post


Link to post
Share on other sites
Quote:
Original post by Antheus
Quote:
I'm using Turbo C++ 3.0


1991 compiler counts as an antique. Can you elaborate on the reason?


I'm in India. Here is my school (most schools actually) we use turbo C++ and learn the obsolete version of C++. (If you want to read a 2 page rant about it go to gamedev.in ) And I'm making a project for school (most people just make dumb things such as a library system...or...things like that). I wanted to make something to the best of my ability (something I can show off later). So I decided to make my own version Of M-Duel (google it).

Share this post


Link to post
Share on other sites
Quote:
Original post by Vish
But this Method too does not work very well as It's quite difficult finding exactly where the memory leak is occuring. And quite often some how the pointer recieved in delete is not present in the memory array. (Don't know why).


Well, let's start with the fact that you don't always get ::operator delete() called with a valid pointer, and sometime's it's not called with a valid pointer. That gives you a clue that there's some logic problem somewhere in your code.

Let's look at a sample of your code.

void operator delete(void *p)
{
if(p == 0)
return;

Logger * Log = Logger::GetInstance();
Log->RemoveMemoryEntry(p);

if(p) // This condition will ALWAYS return true.
free(p);
p = 0; // This statement has no useful effect!
}
The two commented lines indicate you may not have a rigourous grasp of pointers. Make sure you understand why removing them completely would not alter the program at all (in fact the compiler will probably optimize them away, but that's not the point).

I'm afraid that short of stepping through everything with the debugger (break on ::operator delete() and walk the stack), your best bet to locate the cause of your memory problems would be write to your log before each and every delete (or at least key deletes) until you've located the line of code that calls delete with an invalid address. That won't help you find the leak, but if I were a betting man my money would be on the leak being in the same code as the invalid deletion.

Also, remove the first if block in your delete above and see if you get unexpected results.

Your best bet would be to simply refactor the code and use modern C++ idioms -- yes, RAII works with ARM C++ as well as ISO C++. Google RAII, learn it, apply it to your pointers.

--smw

Share this post


Link to post
Share on other sites

if(p == 0)
return;

if(p) // This condition will ALWAYS return true.
free(p);
p = 0; // This statement has no useful effect!

Quite right. I forgot I had already add if(p==0) return; and then I readded the checker later. And the last line I have no idea how that got there. I must have put it earlier for something else and then forgoten to remove it.

I should clean up my code before posting it.

Either way those lines should not really make much of a difference (they would just affect the speed .. slightly. In the long run)

Quote:

Your best bet would be to simply refactor the code and use modern C++ idioms -- yes, RAII works with ARM C++ as well as ISO C++. Google RAII, learn it, apply it to your pointers.

I know what RAII is and I am currently it in all my Classes. But I'm still getting memory leaks. (Mainly due to the fact that if sometimes memory allocation fails. Not all destructors are called)

Another reason why I was getting memory leaks (now not that much) was cause I did not expect size_t to be implemented as an unsigned int ( 16 bit ). Causing all my memory requests above 65k to not work properly (meaning they result in giving me drastically less amount of memory. Which causes me to write data on unknown data. Which was causing it to crash.

If you read the Log file you will notice. That a good amount of, 1024 bytes, of memory has been initialized by something (I don't know I have not called it I've checked) and It is not deallocated. Could you explain why this is happening ? I tried only running the program with the logger yet those 7 object still got initialised and not deleted.
Quote:

Make sure you understand why removing them completely would not alter the program at all (in fact the compiler will probably optimize them away, but that's not the point).

I'm sorry I don't understand. Removing what exactly ? Pointers ? How am I supposed to dynamically allocate memory then ?

Till then I'll manually go through the code debugging it. It sort of din't occur to me to set a break point on the operator new & delete. And I was wondering why the compiler wasn't going inside the operators while debugging line by line.

Share this post


Link to post
Share on other sites
Quote:
Original post by Vish
If you read the Log file you will notice. That a good amount of, 1024 bytes, of memory has been initialized by something (I don't know I have not called it I've checked) and It is not deallocated. Could you explain why this is happening ? I tried only running the program with the logger yet those 7 object still got initialised and not deleted.


It's possible that you're seeing objects in the C++ runtime library being allocated and deallocated. I don't know about the runtime you're using (it's prestandard, so its behaviour is unpredictable without reading its code) but the standard library I/O objects like cout and cin have buffers that get allocated, likely using new, and it's possible they don't get allocated before your logger goes out of scope. Also, your logger itself looks like it might be allocating memory.

Is it possible that those uses could account for the mysterious memory leaks?

--smw

Share this post


Link to post
Share on other sites

This topic is 3728 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this