Code analysis C26408 — Replacing the m_pszHelpFilePath variable in InitInstance

Question

In my application's InitInstance function, I have the following code to rewrite the location of the CHM Help Documentation:

CString strHelp = GetProgramPath();
strHelp += _T("MeetSchedAssist.CHM");
free((void*)m_pszHelpFilePath);
m_pszHelpFilePath = _tcsdup(strHelp);

It is all functional but it gives me a code analysis warning:

C26408 Avoid malloc() and free(), prefer the nothrow version of new with delete (r.10).

---

When you look at the official documentation for m_pszHelpFilePath it does state:

If you assign a value to m_pszHelpFilePath, it must be dynamically allocated on the heap. The CWinApp destructor calls free( ) with this pointer. You many want to use the _tcsdup( ) run-time library function to do the allocating. Also, free the memory associated with the current pointer before assigning a new value.

Is it possible to rewrite this code to avoid the code analysis warning, or must I add a __pragma?

Answer 1

Technically, you can take advantage of the fact that new / delete map to usual malloc/free by default in Visual C++, and just go ahead and replace. The portability won't suffer much as MFC is not portable anyway. Sure you can use unique_ptr<TCHAR[]> instead of direct new / delete, like this:

CString strHelp = GetProgramPath();
strHelp += _T("MeetSchedAssist.CHM");
std::unique_ptr<TCHAR[]> str_old(m_pszHelpFilePath);
auto str_new = std::make_unique<TCHAR[]>(strHelp.GetLength() + 1);
_tcscpy_s(str_new.get(), strHelp.GetLength() + 1, strHelp.GetString());
m_pszHelpFilePath = str_new.release();
str_old.reset();

For robustness for replaced new operator, and for least surprise principle, you should keep free / strdup.

If you replace multiple of those CWinApp strings, suggest writing a function for them, so that there's a single place with free / strdup with suppressed warnings.

Answer 2

You could (should?) use a smart pointer to wrap your reallocated m_pszHelpFilePath buffer. However, although this is not trivial, it can be accomplished without too much trouble.

First, declare an appropriate std::unique_ptr member in your derived application class:

class MyApp : public CWinApp // Presumably
{
// Add this member...
public:
    std::unique_ptr<TCHAR[]> spHelpPath;
// ...
};

Then, you will need to modify the code that constructs and assigns the help path as follows (I've changed your C-style cast to an arguably better C++ cast):

// First three (almost) lines as before ...
CString strHelp = GetProgramPath();
strHelp += _T("MeetSchedAssist.CHM");
free(const_cast<TCHAR *>(m_pszHelpFilePath));

// Next, allocate the shared pointer data and copy the string...
size_t strSize = static_cast<size_t>(strHelp.GetLength() + 1); 
spHelpPath std::make_unique<TCHAR[]>(strSize);
_tcscpy_s(spHelpPath.get(), strHelp.GetString()); // Use the "_s" 'safe' version!

// Now, we can use the embedded raw pointer for m_pszHelpFilePath ...
m_pszHelpFilePath = spHelpPath.get();

So far, so good. The data allocated in the smart pointer will be automatically freed when your application object is destroyed, and the code analysis warnings should disappear. However, there is one last modification we need to make, to prevent the MFC framework from attempting to free our assigned m_pszHelpFilePath pointer. This can be done by setting that to nullptr in the MyApp class override of ExitInstance:

int MyApp::ExitInstance()
{
    // <your other exit-time code>
    m_pszHelpFilePath = nullptr;
    return CWinApp::ExitInstance(); // Call base class
}

However, this may seem like much ado about nothing and, as others have said, you may be justified in simply supressing the warning.