Making Your C++ Code Robust

Introduction

When your C++ program grows in size, it becomes difficult to keep track of its components, and it's easier to introduce bugs. So it is important to realize that some rules should be followed to write code more tolerant to errors.

This article provides some tips that may help you to write code more tolerant to critical errors (crashes). Even if your program currently doesn't follow any coding rules because of its complexity and because of your project's team structure, following the simple rules listed below may help you avoid the majority of crash situations.

Background

For some softwares I develop, I have been using an Open-Source error reporting library called CrashRpt for a long time. The CrashRpt library allows to automatically submit error reports from installations of my software on users' computers. This way all critical errors (exceptions) in my software are automatically reported to me via the Internet, allowing me to improve my software and make users happier with each hot fix release.

While analyzing incoming error reports, I found that often the reason for program crashes can be easily avoided by following simple rules. For example, sometimes I forget to initialize a local variable and it might contain trash causing an array index overflow. Sometimes I don't perform a NULL check before using a pointer variable and this causes an access violation.

I have summarised my experience in several coding rules below that may help you to avoid such mistakes and make your software more stable (robust).

Initializing Local Variables

Non-initialized local variables are a common reason for program crashes. For example, see the following code fragment:

// Define local variables
BOOL bExitResult; // This will be TRUE if the function exits successfully
FILE* f; // Handle to file
TCHAR szBuffer[_MAX_PATH];   // String buffer

// Do something with variables above...

The code fragment above can be a potential reason for a crash, because non of the local variables are initialized. When your code runs, these variables will contain some trash values. For example, the bExitResult boolean variable may contain the value -135913245 (not boolean by nature). Or the szBuffer string variable may not be zero-terminated as it must be. So initializing local variables is very important.

The correct code would be the following:

// Define local variables
  
// Initialize function exit code with FALSE to indicate failure assumption
BOOL bExitResult = FALSE;
// This will be TRUE if the function exits successfully

// Initialize file handle with NULL
FILE* f = NULL; // Handle to file
// Initialize string buffer with empty string
TCHAR szBuffer[_MAX_PATH] = _T("");   // String buffer
// Do something with variables above...

Note: One may say that for some time-critical calculations, variable initialization may be costly, and they are right. If your code should execute as rapidly as possible, you may skip variable initialization at your own risk.

Initializing WinAPI Structures

Many WinAPI functions receive/return parameters through C structures. Such a structure, if incorrectly initialized, may be the reason for a crash. It is recommended to use the ZeroMemory() macro or the memset() function to fill structures with zeroes (this typically sets the structure fields to their default values).

Many WinAPI structures also have the cbSize parameter that must be initialized with the size of the structure before using.

The following code shows how to initialize a WinAPI structure:

NOTIFYICONDATA nf; // WinAPI structure
memset(&nf,0,sizeof(NOTIFYICONDATA)); // Zero memory
nf.cbSize = sizeof(NOTIFYICONDATA); // Set structure size!
// Initialize other structure members
nf.hWnd = hWndParent;
nf.uID = 0;   
nf.uFlags = NIF_ICON | NIF_TIP;
nf.hIcon = ::LoadIcon(NULL, IDI_APPLICATION);
_tcscpy_s(nf.szTip, 128, _T("Popup Tip Text"));

// Add a tray icon
Shell_NotifyIcon(NIM_ADD, &nf);

But! Do not use ZeroMemory() or memset() for your C++ structures that contain objects as structure members; that may corrupt their internal state and be the reason for a crash.

// Declare a C++ structure
struct ItemInfo
{
  // The structure has std::string object inside
  std::string sItemName;
  int nItemValue;
}; 

// Init the structure
ItemInfo item;
// Do not use memset()! It can corrupt the structure
// memset(&item, 0, sizeof(ItemInfo));
// Instead use the following
item.sItemName = "item1";
item.nItemValue = 0;

It is even better to use a constructor for your C++ structure that would init its members with default values:

// Declare a C++ structure
struct ItemInfo
{
    // Use structure constructor to set members with default values
    ItemInfo()
    {
      sItemName = _T("unknown");
      nItemValue = -1;
    }
        
    std::string sItemName; // The structure has std::string object inside
    int nItemValue;
};
// Init the structure
ItemInfo item;
// Do not use memset()! It can corrupt the structure
// memset(&item, 0, sizeof(ItemInfo));
// Instead use the following
item.sItemName = "item1";
item.nItemValue = 0;

Validating Function Input

It is recommended to always validate function input parameters. For example, if your function is a part of the public API of a dynamic link library, and it may be called by an external client, it is not guaranteed that the external client will pass you the correct parameters.

For example, let's look at the hypothetical DrawVehicle() function that draws a sports car on a window with a varying quality. The drawing quality nDrawingQaulity may vary from 0 (coarse quality) to 100 (fine quality). The drawing rectangle in prcDraw defines where to draw the car.

Below is the code. Note how we validate the input parameter values before using them.

BOOL DrawVehicle(HWND hWnd, LPRECT prcDraw, int nDrawingQuality)
{
    // Check that window is valid
    if(!IsWindow(hWnd))
      return FALSE;

    // Check that drawing rect is valid
    if(prcDraw==NULL)
      return FALSE;

    // Check drawing quality is valid
    if(nDrawingQuality<0 || nDrawingQuality>100)
      return FALSE;
   
    // Now it's safe to draw the vehicle

    // ...

    return TRUE;
}

Validating Pointers

Using a pointer without validation is very common. I would even say this is the main reason for crashes in my software.

If you use a pointer, make sure it is not equal to NULL. If your code tries to use a NULL pointer, this may result in an access violation exception.

CVehicle* pVehicle = GetCurrentVehicle();
  
// Validate pointer
if(pVehicle==NULL)
{
    // Invalid pointer, do not use it!
    return FALSE;
}

// Use the pointer

Initializing Function Output

If your function creates an object and returns it as a function parameter, it is recommended to initialize the pointer with NULL in the beginning of the function body.

If you do not explicitly initialize the output parameter and further it is not set due to a bug in function logics, the caller may use the invalid pointer which would possibly cause a crash.

Here is an example of incorrect code:

int CreateVehicle(CVehicle** ppVehicle)
{
    if(CanCreateVehicle())
    {
      *ppVehicle = new CVehicle();
      return 1;
    }    

    // If CanCreateVehicle() returns FALSE,
    // the pointer to *ppVehcile would never be set!
    return 0;
}

The correct code:

int CreateVehicle(CVehicle** ppVehicle)
{
    // First initialize the output parameter with NULL
    *ppVehicle = NULL;

    if(CanCreateVehicle())
    {
      *ppVehicle = new CVehicle();
      return 1;
    }    

    return 0;
}

Cleaning Up Pointers to Deleted Objects

Assign NULL to a pointer after freeing (or deleting) it. This will help to ensure no one will try to reuse an invalid pointer. As you may suppose, accessing a pointer to a deleted object results in an access violation exception.

The following code example shows how to clean up a pointer to a deleted object.

// Create object
CVehicle* pVehicle = new CVehicle();

delete pVehicle; // Free pointer
pVehicle = NULL; // Set pointer with NULL

Cleaning Up Released Handles

Assign NULL (or zero, or some default value) to a handle after freeing it. This will help ensure no one will try to reuse an invalid handle.

Below is an example of how to clean up a WinAPI file handle:

HANDLE hFile = INVALID_HANDLE_VALUE; 
  
// Open file
hFile = CreateFile(_T("example.dat"), FILE_READ|FILE_WRITE, FILE_OPEN_EXISTING);
if(hFile==INVALID_HANDLE_VALUE)
{
    return FALSE; // Error opening file
}

// Do something with file

// Finally, close the handle
if(hFile!=INVALID_HANDLE_VALUE)
{
    CloseHandle(hFile);   // Close handle to file
    hFile = INVALID_HANDLE_VALUE;   // Clean up handle
}

Below is an example of how to clean up a FILE* handle:

// First init file handle pointer with NULL
FILE* f = NULL;

// Open handle to file
errno_t err = _tfopen_s(_T("example.dat"), _T("rb"));
if(err!=0 || f==NULL)
return FALSE; // Error opening file

// Do something with file

// When finished, close the handle
if(f!=NULL) // Check that handle is valid
{
    fclose(f);
    f = NULL; // Clean up pointer to handle
}

Using the delete [] Operator for Arrays

If you allocate a single object with the operator new, you should free it with the operator delete.

But if you allocate an array of objects with the operator new, you should free this array with delete [].

// Create an array of objects
CVehicle* paVehicles = new CVehicle[10];

delete [] paVehicles; // Free pointer to array
paVehicles = NULL; // Set pointer with NULL

or:

// Create a buffer of bytes
LPBYTE pBuffer = new BYTE[255];

delete [] pBuffer; // Free pointer to array
pBuffer = NULL; // Set pointer with NULL

Allocating Memory Carefully

Sometimes it is required to allocate a buffer dynamically, but the buffer size is determined at run time. For example, if you need to read a file into memory, you allocate a memory buffer whose size is equal to the size of the file. But before allocating the buffer, ensure that 0 (zero) bytes are not allocated using malloc() or new. Passing the incorrect parameter to malloc() results in a C run-time error.

The following code example shows dynamic buffer allocation:

// Determine what buffer to allocate.
UINT uBufferSize = GetBufferSize(); 

LPBYTE* pBuffer = NULL; // Init pointer to buffer

// Allocate a buffer only if buffer size > 0
if(uBufferSize>0)
    pBuffer = new BYTE[uBufferSize];

For additional tips on how to allocate memory correctly, you can read the Secure Coding Best Practices for Memory Allocation in C and C++ article.

Using Asserts Carefully

Asserts can be used in debug mode for checking pre-conditions and post-conditions. But when you compile your program in Release mode, asserts are removed on the pre-processing stage. So, using asserts is not enough to validate your program's state.

Incorrect code:

#include <assert.h>
  
// This function reads a sports car's model from a file
CVehicle* ReadVehicleModelFromFile(LPCTSTR szFileName)
{
    CVehicle* pVehicle = NULL; // Pointer to vehicle object

    // Check preconditions
    assert(szFileName!=NULL); // This will be removed by preprocessor in Release mode!
    assert(_tcslen(szFileName)!=0); // This will be removed in Release mode!

    // Open the file
    FILE* f = _tfopen(szFileName, _T("rt"));

    // Create new CVehicle object
    pVehicle = new CVehicle();

    // Read vehicle model from file

    // Check postcondition 
    assert(pVehicle->GetWheelCount()==4); // This will be removed in Release mode!

    // Return pointer to the vehicle object
    return pVehicle;
}

As you can see in the code above, usage of asserts can help you to check your program state in Debug mode, but in Release mod,e these checks will just disappear. So, in addition to asserts, you have to use if() checks.

The correct code would be:

#include <assert.h>
  
CVehicle* ReadVehicleModelFromFile(LPCTSTR szFileName, )
{
    CVehicle* pVehicle = NULL; // Pointer to vehicle object

    // Check preconditions
    assert(szFileName!=NULL);
    // This will be removed by preprocessor in Release mode!

    assert(_tcslen(szFileName)!=0);
    // This will be removed in Release mode!

    if(szFileName==NULL || _tcslen(szFileName)==0)
      return NULL; // Invalid input parameter

    // Open the file
    FILE* f = _tfopen(szFileName, _T("rt"));

    // Create new CVehicle object
    pVehicle = new CVehicle();

    // Read vehicle model from file

    // Check postcondition 
    assert(pVehicle->GetWheelCount()==4); // This will be removed in Release mode!

    if(pVehicle->GetWheelCount()!=4)
    { 
      // Oops... an invalid wheel count was encountered!  
      delete pVehicle; 
      pVehicle = NULL;
    }

    // Return pointer to the vehicle object
    return pVehicle;
}

Checking Return Code of a Function

It is a common mistake to call the function and assume it will succeed. When you call a function, it is recommended to check its return code and values of output parameters.

The following code calls functions in succession. Whether to proceed or to exit depends on return code and output parameters.

HRESULT hres = E_FAIL;
IWbemServices *pSvc = NULL;
IWbemLocator *pLoc = NULL;

hres =  CoInitializeSecurity(
    NULL, 
    -1,                          // COM authentication
    NULL,                        // Authentication services
    NULL,                        // Reserved
    RPC_C_AUTHN_LEVEL_DEFAULT,   // Default authentication 
    RPC_C_IMP_LEVEL_IMPERSONATE, // Default Impersonation  
    NULL,                        // Authentication info
    EOAC_NONE,                   // Additional capabilities 
    NULL                         // Reserved
    );

if (FAILED(hres))
{
    // Failed to initialize security
    if(hres!=RPC_E_TOO_LATE) 
       return FALSE;
}
    
hres = CoCreateInstance(
    CLSID_WbemLocator,
    0, 
    CLSCTX_INPROC_SERVER, 
    IID_IWbemLocator, (LPVOID *) &pLoc);

if (FAILED(hres) || !pLoc)
{
    // Failed to create IWbemLocator object. 
    return FALSE;
}

hres = pLoc->ConnectServer(
     _bstr_t(L"ROOT\\CIMV2"), // Object path of WMI namespace
     NULL,                    // User name. NULL = current user
     NULL,                    // User password. NULL = current
     0,                       // Locale. NULL indicates current
     NULL,                    // Security flags.
     0,                       // Authority (e.g. Kerberos)
     0,                       // Context object 
     &pSvc                    // pointer to IWbemServices proxy
     );

if (FAILED(hres) || !pSvc)
{
    // Couldn't conect server
    if(pLoc) pLoc->Release();     
    return FALSE;  
}
hres = CoSetProxyBlanket(
   pSvc,                        // Indicates the proxy to set
   RPC_C_AUTHN_WINNT,           // RPC_C_AUTHN_xxx
   RPC_C_AUTHZ_NONE,            // RPC_C_AUTHZ_xxx
   NULL,                        // Server principal name 
   RPC_C_AUTHN_LEVEL_CALL,      // RPC_C_AUTHN_LEVEL_xxx 
   RPC_C_IMP_LEVEL_IMPERSONATE, // RPC_C_IMP_LEVEL_xxx
   NULL,                        // client identity
   EOAC_NONE                    // proxy capabilities 
);
if (FAILED(hres))
{
    // Could not set proxy blanket.
    if(pSvc) pSvc->Release();
    if(pLoc) pLoc->Release();     
    return FALSE;               
}

Using Smart Pointers

If you intensively use pointers to shared objects (e.g., COM interfaces), it is a good practice to wrap them into smart pointers. The smart pointer will take care of your object's reference counting and will protect you from accessing an object that was already deleted. That is, you don't need to worry about controlling the lifetime of your interface pointer.

For additional info on smart pointers, see the following articles: Smart Pointers - What, Why, Which? and Implementing a Simple Smart Pointer in C++.

Below is an example code (borrowed from MSDN) that uses ATL's CComPtr template class as a smart pointer.

#include <windows.h>
#include <shobjidl.h> 
#include <atlbase.h> // Contains the declaration of CComPtr.
int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE, PWSTR pCmdLine, int nCmdShow)
{
    HRESULT hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | 
        COINIT_DISABLE_OLE1DDE);
    if (SUCCEEDED(hr))
    {
        CComPtr<IFileOpenDialog> pFileOpen;
        // Create the FileOpenDialog object.
        hr = pFileOpen.CoCreateInstance(__uuidof(FileOpenDialog));
        if (SUCCEEDED(hr))
        {
            // Show the Open dialog box.
            hr = pFileOpen->Show(NULL);
            // Get the file name from the dialog box.
            if (SUCCEEDED(hr))
            {
                CComPtr<IShellItem> pItem;
                hr = pFileOpen->GetResult(&pItem);
                if (SUCCEEDED(hr))
                {
                    PWSTR pszFilePath;
                    hr = pItem->GetDisplayName(SIGDN_FILESYSPATH, &pszFilePath);
                    // Display the file name to the user.
                    if (SUCCEEDED(hr))
                    {
                        MessageBox(NULL, pszFilePath, L"File Path", MB_OK);
                        CoTaskMemFree(pszFilePath);
                    }
                }
                // pItem goes out of scope.
            }
            // pFileOpen goes out of scope.
        }
        CoUninitialize();
    }
    return 0;
}

Using == Operator Carefully

Look at the following code fragment:

CVehicle* pVehicle = GetCurrentVehicle();
   
// Validate pointer
if(pVehicle==NULL) // Using == operator to compare pointer with NULL
   return FALSE; 

// Do something with the pointer
pVehicle->Run();

The code above is correct and uses pointer validation. But assume you made a typing mistake and used an assignment operator (=) instead of the equality operator (==):

CVehicle* pVehicle = GetCurrentVehicle();

// Validate pointer
if(pVehicle=NULL) // Oops! A mistyping here!
 return FALSE; 

// Do something with the pointer
pVehicle->Run(); // Crash!!!

As you can see from the code above, such mistyping may be the result of a stupid crash.

Such an error can be avoided by slightly modifying the pointer validation code (exchange left side and right side of the equality operator). If you mistype in such a modified code, it will be detected on compilation stage.

// Validate pointer
if(NULL==pVehicle)
// Exchange left side and right side of the equality operator
    return FALSE; 

// Validate pointer
if(NULL=pVehicle)
// Oops! A mistyping here! But the compiler returns an error message.
    return FALSE;

History

• 24^th July 2011 - Initial release.