Procedures for Safe Coding

Overview

One of the things every programmer learns as he or she gains experience is how to avoid mistakes.  Each time you make a mistake, you surely consider a way to avoid that mistake next time.  As you gain experience, you start to discover practices that can help you to avoid a lot of mistakes.  But of course no matter how advanced you ever become, you will still make mistakes.  Anyone who claims to be impervious to mistakes is a fool.  And the irony is that sometimes you get so good at avoid mistakes, some of the simplest most idiotic mistakes can keep you busy for hours just because you follow so many  practices that do work, you didn’t consider that you made such a trivial error and instead spend your time searching for a more complicated mistake.

So the first rule I wish to present is to stay humble and never consider that you are impervious to mistakes, no matter what safety guidelines you follow.  I am talking to some of you pros out there (because beginners always assume they are the source of the problem).

Memory Management

Throughout the entirety of this site I always assume C++, and to a lesser degree C.  This section does not apply if you are using Java or C# (and managed friends).

In the best-case scenario you will have some kind of memory manager in place that will alert you to leaked memory.  In L. Spiro Engine, the engine is designed to be used in “states” that are completely modular and have no knowledge of other states.  The main menu, options menu, gameplay area, bonus round, etc., are examples of states.  A mechanism is provided for passing data from one state to another, but programmers are allowed to work within their states alone, and feel secure that things done by other programmers are not affecting their own code (avoid singletons, please).  Under this condition it can be asserted that, if one person makes a state that allocates some RAM, and deallocates all of that RAM when leaving the state, then no leaks have occurred.  The engine will report any discrepancies under this guideline.  If the layout of the RAM was not exactly the same before and after the state ran, something was leaked and an error message is reported.  This has helped me to discover and fix some leaks in my engine too, and will later help others using my engine.

However, it can obviously not be assumed that everyone will be using a customized memory manager.  So I will present a few ways to help the average user to avoid leaks.

• Firstly, use smart pointers.  If you are exclusively using smart pointers, you should be quite well off.
• But let’s say you are not ready to link to boost or don’t want to use C++11, or maybe you just can’t.  There is a very simple way to use new and delete effectively.  I mentioned in a previous article that adding a bunch of new’s and then later going back to add a bunch of delete’s is a way to shoot yourself in the foot (and get fired).  Every time you add a new (or malloc()), stop and go write a delete (or free()).
• The above tip can be made pretty easy on yourself.  Make a function whose job is to fully release/deallocate all resources made by your class and set the pointers/handles to NULL/invalid, so that if the function is called twice in a row nothing will happen.  Basically, this gives you a centralized location where all of your deallocations take place, so whenever you want to abandon your object (such as due to an initialization error) you can simply call this function and be confident that all buffers that were allocated will be freed, and those that were not will properly be ignored (and left NULL).  A huge advantage this provides to you is that as your class grows and you add more allocations, you only have to keep track of one location for freeing those resources.  The alternative is that your deallocations are spread to many areas of your class and when you add more allocations you have to be careful not to forget one of the deallocation hotspots.
• Think first, code later.  I generally try to keep my allocations simple.  Usually I have one CreateBlah()* function and a Reset() function (which follows the advice of the previous tip, and is called by the destructor so you don’t have to call it manually, ever).  But there are times when there is one main allocation routine and several different deallocation routines.  Think about every possible flow that could cause a deletion of your resources and handle them with care.  If you followed my advice, that just means adding a call to Reset() (for example) at various locations in your code.  If you did not, you need a sturdy plan in your head.  If you have a sturdy plan in your head, you may not necessarily need to add every delete at the same time you code a new, but if you don’t then you must definitely make a mental note that you are missing a delete somewhere.  A note that will bug you until you remember what it was.
• As mentioned by ApochPiQ: RAII.  RAII is a way of guaranteeing resources are freed or unlocked when the function returns (technically when scope is lost, but I will get into that later) or when exceptions are thrown.  Basically, you wrap the acquisition and release of resources to constructors and destructors of classes.  In L. Spiro Engine, all mutexes or critical sections (depending on the build target) are locked and unlocked via the RAII paradigm.  When I want to lock a critical section, I pass the critical section object to the constructor of a class.  That class keeps a pointer to my critical section and calls EnterCriticalSection() on it.  The destructor of that class calls LeaveCriticalSection() on it.  In this way I am guaranteed never to forget leaving the critical section, no matter what happens.  The class that enters and leaves the critical section is stored locally on the stack, so its destructor will be called automatically when its scope expires (which in this case is usually when the function returns) and also when exceptions are thrown.  The alternative is that I would have to manually remember to leave the critical section before each return and also before propagating exceptions, which is just asking for trouble.  Additionally, virtually all temporary memory allocations are handled via game-ready versions of std::vector.  When unpacking model files or performing image filtering, for example, all allocations are handled via this class and thus are guaranteed to be released at appropriate times.

* I used to make a class and then a function just called Create(), because I figured if you knew the class then you knew what you were creating.  But Microsoft® Visual Studio® kept showing me a list of 15 different classes with Create() methods when I clicked one of them and tried to “Go to Definition”, so I finally decided to redundantly add functions named for what they were creating, even though it was already obvious by the type of class being created.

Stack Bugs
The stack is basically the active thread’s set of local data.  Every thread has its own stack.  Every variable you create inside a function goes onto the stack, and every parameter you pass to a function also goes onto the stack.  The stack is a finite amount of memory given to each thread, so if you try to create a variable inside a function that is an array of some thousand elements you are likely to see a “stack overflow” exception when you execute the application.  Likewise if you ever enter into an infinitely recursive function, you definitely will receive such an error, but you may also encounter this error if you have a recursive function of variable depth.  For instance, if you make a new language, and naively parse that language using recursive functions, you will not encounter this error for a while but eventually someone who has made an extremely large script will eventually encounter this error.
These types of things can cause stack overflow, but there are other things that can cause corruption of the stack also.

• Buffer overflows.  This is the most common form of stack corruption.  For that reason, Microsoft’s compilers are designed to detect such corruption in debug mode.  In order to accomplish this type of stack corruption, you only need to copy more variables to a local array than the local array was programmed to accept.  While this seems trivial, this is the single most common method of attack hackers use to take over computers.  By discovering that their additional data will cause an unexpected area of RAM to be overwritten, hackers are able to exploit the buffer overflow to allow them to copy some bytes into your executable that, if executed, will produce immeasurably undesired results.  Because all machine code is in the end just a series of bytes, they can implant an immeasurable amount of code that can do whatever they please.
• For the above problem, never use strings as just raw character pointers.  Either wrap them around std::string or make your own game-ready implementations of std::string (no, std is not game-ready).  If you become comfortable enough with this security flaw, you may be allowed to make a few functions that accept const char *, but you really need to understand the underlying mechanics for you to ever allow yourself to do this.
• And let us assume you opt for std::string.  Pass it as const std::string & instead of std::string.  There is no reason to copy the whole string when you want to pass it to a function.
• Another way in which the stack could be corrupted is by writing to a pointer that was ill-defined.  If a pointer happens to point to an address that is on the stack, and you write to it, you have corrupted the stack.  Luckily, however, this a very rare case, and since the stack usually changes each time you run the application, sometimes you will get a stack problem, but other times you will get an invalid access.  If you get both of these errors, most likely you are writing to an invalid pointer.
• Never take for granted functions such as sprintf().  In Microsoft® Visual Studio® there is a safer version called s_sprintf().  If your development platform has no such function, make one.  These functions exist for a reason.  Using the standard set of functions can cause your stack to corrupt even if you are not directly corrupting it yourself.
• Probably the single most common mistake I see is people using szFilePath[256] instead of szFilePath[MAX_PATH].  Guess what.  The maximum file-path length is not 256.  It is MAX_PATH (260) on Windows® and PATH_MAX on Macintosh OS X 5.0 and above (1,024).  Hard-coding the length of the string is just asking for buffer overflows and security flaws.  Always rely on the appropriate macros to determine the safest size of a string intended to hold a file path.

Pointers

Another extremely common mistake I see people making is casting pointers to  DWORD or unsigned long or unsigned int, etc.  What is the size of an int?  What is the size of a long?  What is the size of a pointer?  If you gave anything but “it depends” to any of these questions you have answered incorrectly.  All of them can change sizes depending on the compiler and the target platform.  If you need to do standard math on a pointer for whatever reason you may have, use a type that has been specifically defined to be the same size as a pointer under all compilers and platforms.  The Microsoft® Windows® API provides a typedef named UINT_PTR.  As its name suggests, it is an unsigned integer that has the same size as a pointer, regardless of whether you are targeting 32-bit Windows® or 64-bit.

Just as fun trivia, I will also mention that pointers are usually considered signed values.  See for yourself (code assumes 32-bit pointers):

if ( (void *)0xFFFFFFFF > (void *)0x7FFFFFFF ) { cout << "Must be unsigned." << endl; }
else { cout << "Must be signed." << endl; }

And Remember
There are a few gotchas that can cause you to leak memory (etc.) even if you followed all of the above practices.  Examine the following classes:

class CBase {
public :
	CBase() {
		m_pucPleaseRlsMe = new unsigned char[1000];
	}
	virtual ~CBase() {
		Reset();
	}

	virtual void		Reset() {
		if ( m_pucPleaseRlsMe ) {
			delete [] m_pucPleaseRlsMe;
			m_pucPleaseRlsMe = NULL;
		}
	}

protected :
	unsigned char *		m_pucPleaseRlsMe;
};

class CInheritFrom : public CBase {
public :
	CInheritFrom() {
		m_pucPleaseRlsMe2 = new unsigned char[2000];
	}

	virtual void		Reset() {
		if ( m_pucPleaseRlsMe2 ) {
			delete [] m_pucPleaseRlsMe2;
			m_pucPleaseRlsMe2 = NULL;
		}
		CBase::Reset();
	}

protected :
	unsigned char *		m_pucPleaseRlsMe2;
};

Can you spot the problem?  If Reset() is called it will correctly delete one or both buffers depending on whether the object is CBase or CInheritFrom.  And CInheritFrom knows that CBase will call Reset() while being destroyed, which will allow CInheritFrom to implicitly release its own resources, right?
Wrong.
Virtual functions do not propagate upwards from base classes during constructors and destructors.  It doesn’t make sense that they should.  By the time the destructor for CBase is being called, the destructor for CInheritFrom has already been called and thus it must be assumed that CInheritFrom is in an invalid state.  Likewise, during constructors, CBase’s is called before CInheritFrom’s, and again CInheritFrom would be in an invalid state if any virtual functions were propagated up into it.  To avoid a leak here, CInheritFrom needs to also call Reset() from its destructor.

There are tons of details to remember when coding safe.  More to come as people suggest things that I can’t remember off the top of my head.

L. Spiro