Troubles with GetOpenFileName()

Hello again, this time for a new problem with the GetOpenFileName function.
Well, I have a function which calls GetOpenFileName to retrieve a path to one to open. The problem is that sometimes, quite often, the program ends with a Access Violation exception during the execution of the common dialog; to be precise, it is after the user clicked the "ok" button, which normally close the box while sending back to the caller the file name, and before the function returns. (or at least nothing that is after the call execute)
I tried my debugger to see what was going wrong but I only saw it once but at that time I had many other errors so I left that one; every other times, the program runs well...

What could it be? What could I do?

Here's some code, maybe it could help.

Code:

---

/* The function takes parameters and returns 0 if no error occured, a negative value otherwise.
_trace and _tracecc are macros expanding to message boxes depending on wether I'm debugging or not.
*/
int opendlg(HWND parent, int type, const _TCHAR *title, _TCHAR *file, const _TCHAR *filt)
{
    OPENFILENAME ofn;

    /* _WIN32_WINNT is defined to 0x0501 */
    ofn.lStructSize        = sizeof(OPENFILENAME);
    ofn.hwndOwner          = parent;
    ofn.hInstance          = NULL;
    ofn.lpstrFilter        = filt;
    ofn.lpstrCustomFilter  = NULL;
    ofn.nFilterIndex        = 1;
    ofn.lpstrFile          = file;
    ofn.nMaxFile            = MAX_LENGTH;  /* A defined constant */
    ofn.lpstrFileTitle      = NULL;

    /* Set default folder to the PERSONAL one. */
    /* _sbuf is a global buffer declared as _TCHAR _sbuf[MAX_LENGTH] */
    if (SHGetFolderPath(NULL, CSIDL_PERSONAL, NULL, 0, _sbuf) != 0) {
        _trace(_T("SHGetFolderPath failed."),
              _T("Cannot retrieve the personal folder."));
        return -1;
    }
    ofn.lpstrInitialDir    = _sbuf;
    ofn.lpstrTitle          = title;
    ofn.Flags              = OFN_EXPLORER | OFN_HIDEREADONLY | OFN_NONETWORKBUTTON
                              | OFN_NOREADONLYRETURN;
    if (type == OPENDLG_OPENFILE)
        ofn.Flags |= OFN_FILEMUSTEXIST;
    else if (type == OPENDLG_SAVEFILE)
        ofn.Flags |= OFN_OVERWRITEPROMPT;

    ofn.lpstrDefExt        = NULL;
    ofn.lCustData          = 0;
    ofn.pvReserved          = NULL;
    ofn.dwReserved          = 0;
    ofn.FlagsEx            = 0;

    /* Retrieve the file name. */
    if ((type == OPENDLG_OPENFILE && !GetOpenFileName(&ofn))
        || (type == OPENDLG_SAVEFILE && !GetSaveFileName(&ofn))) {
        if (CommDlgExtendedError() == 0)
            return 0;

        _tracecc(_T("GetOpenFileName or GetSaveFileName failed."),
                _T("Cannot create Open/Save File dialog box."));

        return -2;
    }

    /* MAKEDWORD(high, low) is designed to OR (|) two WORDs to make a DWORD */
    return MAKEDWORD(ofn.nFileExtension, ofn.nFileOffset);
}

---

MAX_LENGTH should be at least as big as MAX_PATH

Don't bother setting 0's in ofn. Zero out the entire structure then set the non-zero members.

Code:

---

OPENFILENAME ofn = {0};

---

gg

MAX_LENGTH is defined larger than MAX_PATH, I used to use MAX_PATH but sometimes it is not sufficient.
For the zero members, well, I know that I could zero the whole structure but in the case the compiler does not optimize it and I end up with a structure filled twice... I just do things like that all the time. ^^
But have you a possible answer to my question?

Quote:

---

Originally posted by lyx
For the zero members, well, I know that I could zero the whole structure but in the case the compiler does not optimize it and I end up with a structure filled twice...

---

I have no idea what you are trying say. Try the code below and see if your problem goes away.

Code:

---

OPENFILENAME ofn = {0}

---

gg

Sorry, I just said that if you set all members to zero first and then fill non-zero members with a correct value, then you'll end up filling the non-zero members twice if the compiler does not suppress the first useless initialization. Besides I cannot try the code right now, I have been reinstalling Windows so I have to download the Platform SDK again... I'll tell you when I can compile code. Thanks.

You should zero out every Win32 structure you use. Otherwise, you're just asking for trouble.

Check out the MSDN example.

gg

I tried to zero all my structures out before filling them but it did nothing...

Is your filter string double null terminated?

ie

szFilter[255]="Jpeg (*.jpg)\0*.jpg\0\0";

A quoted constant string is already null terminated, so, wouldn't "\0\0" set three null characters?
Though I'll try that.
Anyway, is there a way to make my debugger execute the program in the exact context (memory) as when I got the bug in a normal execution?

[edit]The double null terminator wasn't successful, sadly.

I'll try again with the debugger, hope it will help. Thanks anyway, it didn't help much but. ^^

Hate to ask the obvious, but when everything else fails...

Is file declared as such?

Code:

---

TCHAR file[MAX_LENGTH] = { 0 };

---

Well, quite the same, file is declared as

Code:

---

_TCHAR *file = (_TCHAR *)LocalAlloc(LPTR, sizeof(_TCHAR) * MAX_LENGTH);

---

Which also means that my pointer is filled with zeros.

[edit]I know I should have used the heap management functiosn but it's not the problem right now. ^^

[edit2]What the debugger says is somehow different from what I get from running th program "manualy", it (the debugger) throws an Access Violation exception but only once I've closed the program whereas executing it otherwise results in the application stopping in the middle of the GetOpenFileName() function processing. That's all I know thus far. :/

This test app worked fine for me. Maybe it'll help.

Code:

---

enum
{
    OPENDLG_OPENFILE,
    OPENDLG_SAVEFILE,
};

int opendlg(int type, const TCHAR *title, TCHAR *file, const TCHAR *filt)
{
    OPENFILENAME ofn = {0};
    TCHAR _sbuf[MAX_PATH];
    TCHAR *initDir = _sbuf;

    if (!SUCCEEDED(SHGetFolderPath(NULL, CSIDL_PERSONAL, NULL, 0, _sbuf)))
    {
        initDir = NULL;
    }

    ofn.lStructSize        = sizeof(OPENFILENAME);
    ofn.lpstrFilter        = filt;
    ofn.nFilterIndex        = (filt != NULL ? 1 : 0);
    ofn.lpstrFile          = file;
    ofn.nMaxFile            = MAX_PATH;
    ofn.lpstrInitialDir    = initDir;
    ofn.lpstrTitle          = title;
    ofn.Flags              = OFN_EXPLORER | OFN_HIDEREADONLY |
                              OFN_NONETWORKBUTTON | OFN_NOREADONLYRETURN;

    if (type == OPENDLG_OPENFILE)
    {
        ofn.Flags |= OFN_FILEMUSTEXIST;
    }
    else if (type == OPENDLG_SAVEFILE)
    {
        ofn.Flags |= OFN_OVERWRITEPROMPT;
    }

    if ((type == OPENDLG_OPENFILE && !GetOpenFileName(&ofn)) ||
        (type == OPENDLG_SAVEFILE && !GetSaveFileName(&ofn)))
    {
        if (CommDlgExtendedError() == 0)
        {
            return 0;
        }

        return -1;
    }

    return ofn.nFileExtension + ofn.nFileOffset;
}


int main()
{
    TCHAR filename[MAX_PATH] = {0};
    TCHAR filter[MAX_PATH] =
    {
        TEXT("Text Files\0*.txt\0")
        TEXT("Bin Files\0*.bin\0")
    };
    int res;

    _tprintf(TEXT("WINVER = 0x%04X\n"), WINVER);
    _tprintf(TEXT("_WIN32_WINNT = 0x%04X\n"), _WIN32_WINNT);

    res = opendlg(OPENDLG_OPENFILE, TEXT("*** Test 1 ***"), filename, NULL);
   
    _tprintf(TEXT("res = %d\nfile = %s\n"), res, filename);

    res = opendlg(OPENDLG_OPENFILE, TEXT("*** Test 2 ***"), filename, filter);

    _tprintf(TEXT("res = %d\nfile = %s\n"), res, filename);

    return 0;
}

---

gg

Well, I'm going to re-design the whole project, if I made such an unfindable mistake, it's probably because my project was badly designed from the beginning.
Thanks for all who helped.

Lyx.

>._TCHAR *file = (_TCHAR *)LocalAlloc(LPTR, sizeof(_TCHAR) * MAX_LENGTH);

If you alloc *file a constant size every run, why dynamically alloc it at all?

As to the double terminators

>>wouldn't "\0\0" set three null characters?

Either way, three or two is irrelevant.

Three still being safe.
One creating an access violation.

MSDN says
"lpstrFilter::
Pointer to a buffer containing pairs of null-terminated filter strings. The last string in the buffer must be terminated by two NULL characters.

The first string in each pair is a display string that describes the filter (for example, "Text Files"), and the second string specifies the filter pattern (for example, "*.TXT"). To specify multiple filter patterns for a single display string, use a semicolon to separate the patterns (for example, "*.TXT;*.DOC;*.BAK"). A pattern string can be a combination of valid file name characters and the asterisk (*) wildcard character.

Do not include spaces in the pattern string. "