| crashrpt |
A crash reporting system for Windows applications

# Making Your Code Robust

This page contains some tips on writing C++ code robust to errors. A program that conforms to these rules is less likely to crash.

This page contains the following topics:

- Introduction
- Initializing Local Variables
- Initializing WinAPI Structures
- Validating Function Input
- Validating Pointers
- Initializing Function Output
- Cleaning Up Pointers to Deleted Objects
- Cleaning Up Released Handles
- Using delete [] Operator for Arrays
- Allocating Memory Carefully
- Using Asserts Carefully
- Checking Return Code of a Function
- Using Smart Pointers
- Using == Operator Carefully

## Introduction

As a program grows in size and dependencies, you start losing trace of its components, get further away from the big picture, and ease the introduction of bugs [How to write robust code]. So it is important to realise that some rules should be followed to write a code more tolerant to errors.

Generally speaking, robust code has such features as: it is well designed, neat and tidy, well named, well commented, well tested, it rarely crashes.

If your program follows consistent coding rules (for example, C++ Programming Style Guidelines, Programming in C++, Rules and Recommendations or Google C++ Style Guide) it is less likely to crash, because it has a consistent structure.

Even if your program currently doesn't follow any coding rules because of its complexity and because of your team structure, following the simple rules listed below may help you to avoid the majority of the crash situations.

## Initializing Local Variables

Not ininitialized local variables are a common reason of 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 of a crash, because none of local variables is initialized. 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...

## Initializing WinAPI Structures

Many WinAPI functions receive/return parameters through C structures. Such a structure, if incorrectly initialized, may be the reason of a crash.

It is recommended to use ZeroMemory() or memset() to fill the structure with zeroes (this typically sets 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;
      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 of a crash.

      // Declare a C++ structure
      struct ItemInfo
      {
        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;

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.

      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

If you use a pointer, make sure it is not equal to NULL.

      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 invalid pointer which would possibly cause a crash.

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 noone will try to reuse an invalid pointer.

     // 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 something else default value) to a handle after freeing it. This will help to ensure noone 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 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

Ensure that 0 (zero) bytes are not allocated using malloc() or new.

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

      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 preconditions and postconditions. But when you compile your program in release mode, asserts are removed on the preprocessing stage. So, using asserts is not enough to validate your program's state.

Incorrect code:

      #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 by preprocessor 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 by preprocessor 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 mode these checks will just disappear.

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 by preprocessor 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 by preprocessor 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))
        {
            log_write(_T("Failed to initialize security. Error code = %X\n"), hres);
            if(hres!=RPC_E_TOO_LATE) return FALSE;
        }

        hres = CoCreateInstance(
            CLSID_WbemLocator,
            0,
            CLSCTX_INPROC_SERVER,
            IID_IWbemLocator, (LPVOID *) &pLoc);

        if (FAILED(hres) || !pLoc)
        {
            log_write(_T("Failed to create IWbemLocator object. Err code = %X"), hres);
            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)
        {
            log_write(_T("Couldn't conect server\n"));
            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))
        {
            log_write(_T("Could not set proxy blanket.\n"));
            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 mistyping and used an assignment operator (=) instead of equality operator (==):

      CVehicle* pVehicle = GetCurrentVehicle();

      // Validate pointer
      if(pVehicle=NULL) // Oop! 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 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 modifyed code, such mistyping 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) // Oop! A mistyping here! But the compiler returns an error message.
         return FALSE;