CString.GetBuffer memory leaks

[dem7w2]

After plenty of google searches and experimental code, I feel defeated. I'm getting memory corruption on the heap when I run the following code:

CFileDialog fileDlg
    (
    FALSE,
    NULL,
    NULL,
    OFN_OVERWRITEPROMPT,
    "Ignore this line (*.txt, *.xml)|*.txt;*.xml||"
    );

CString fileName = getDefaultFileName(false); //returns a CString. it also uses
                                              //CString Replace and Remove functions.
fileDlg.m_ofn.lpstrFile = fileName.GetBuffer(); //I've tried different params
//for GetBuffer such as 0, fileName.GetLength(), and fileName.GetLength()+100

if(IDOK == fileDlg.DoModal())
    {
    fileName.ReleaseBuffer();
    save_file( fileDlg.GetPathName(), false );
    }

Without lines 10, 12, and 17 the program runs fine. There's no specific line that the memory corruption error comes up. Sometimes it's not noticed until save_file tries to initialize some variables.

*Just tried something that worked* Turns out, if I use fileName.GetLength()+500 for the param, it works fine. lpstrFile gets changed to the full path after DoModal() runs, so it needed extra memory allocated. But is there a safer way to do this?

EDIT: Is there a way to do this without GetBuffer at all?

[andywestken]

Your code is setting the lpstrFile (to point at the buffer obtained with GetBuffer) BUT it's not setting the corresponding nMaxFile member, which must be set to the size of the buffer pointed to by lpstrFile.

According to MSDN, the buffer should be at least 256 characters long (see MSDN notes on nMaxFile in entry for OPENFILENAME structure.) But most people use _MAX_PATH here, which is 260, a #define provided by the CRT for this purpose.

fileDlg.m_ofn.lpstrFile = fileName.GetBuffer(_MAX_PATH);
fileDlg.m_ofn.nMaxFile = _MAX_PATH;

BUT another part of the problem is that you were reinventing the wheel. The MFC automatically handles the initial filename if you pass it as the third parameter of the constructor call. They set up an internal buffer and the length during construction. So your crash was probably because you were changing lpstrFile to point to a smaller buffer but not altering nMaxFile, which was still set to the default 260 chars. Of course, when you made your buffer longer than 260, the problem went away.

So, if you let MFC do more of the work, you end up with:

CString fileName = getDefaultFileName(false); //returns a CString. it also uses
                                              //CString Replace and Remove functions.
CFileDialog fileDlg
    (
    FALSE,
    NULL,
    fileName, // let MFC know the initial filename (they copy it into their buffer)
    OFN_OVERWRITEPROMPT,
    _T("Ignore this line (*.txt, *.xml)|*.txt;*.xml||")
    );

if(IDOK == fileDlg.DoModal())
    {
    save_file( fileDlg.GetPathName(), false );
    }

Which doesn't need either GetBuffer or ReleaseBuffer to be used!

Andy

PS The only time I have seen an external buffer provided to CFileDialog is when its being used in multi-select mode, in which case you need more space than 260 chars.

[dem7w2]

you were reinventing the wheel

Well said! That's a shame I didn't notice that, as it would've saved me hours. Nevertheless, it works now. As a side note, using _MAX_PATH also works (and nMaxFile is set to _MAX_PATH by default).

Thanks Andy!