Big problem with GetOpenFileName().

I have had this problem, after I call DoFileDialog() a second time, and I hover over a file, my whole program terminates with no warning or error message.

Code:

---

int DoFileDialog(HWND hwnd, BOOL bSave)
{
  char *pszTempPath = new char[MAX_PATH];
  OPENFILENAME ofn;
  ZeroMemory(pszTempPath, MAX_PATH);
  ZeroMemory(szPath, MAX_PATH);
  ZeroMemory(&ofn, sizeof(ofn));
  ofn.lStructSize = sizeof(ofn);
  ofn.hwndOwner = hwnd;
  ofn.lpstrFilter = "Bitmap Files\0*.bmp\0All Files\0*.*\0\0";
  ofn.lpstrFile = pszTempPath;
  ofn.nMaxFile = MAX_PATH;
  ofn.lpstrDefExt = "*.bmp";
  ofn.Flags = OFN_PATHMUSTEXIST | OFN_OVERWRITEPROMPT;
  ofn.FlagsEx = OFN_EX_NOPLACESBAR;
  BOOL bRet;
  if(bSave)  bRet = GetSaveFileName(&ofn);
  else  bRet = GetOpenFileName(&ofn);
  if(bRet)
  return FALSE;
  strcpy(szPath, pszTempPath);
  delete [] pszTempPath;
  return TRUE;
}

---

After searching this board and google I find out that this is a common problem, however, those people always fix their problems by fixing a buffer overflow problem. I'm smart enough to avoid that from the beggining. So please help! Why isn't this working more than once??

Code:

---

delete[] pszTempPath;

---

New[] works only with delete[].

Okay, fixed. But the problem still remains.

Code:

---

  if(bRet)
  {
      delete [] pszTempPath;
      return FALSE;
  }

---

Code:

---

int DoFileDialog(HWND hwnd, BOOL bSave)
{
  char *pszTempPath = new char[MAX_PATH];
  OPENFILENAME ofn;
  ZeroMemory(pszTempPath, MAX_PATH);
  ZeroMemory(szPath, MAX_PATH);
  ZeroMemory(&ofn, sizeof(ofn));
  ofn.lStructSize = sizeof(ofn);
  ofn.hwndOwner = hwnd;
  ofn.lpstrFilter = "Bitmap Files\0*.bmp\0All Files\0*.*\0\0";
  ofn.lpstrFile = pszTempPath;
  ofn.nMaxFile = MAX_PATH;
  ofn.lpstrDefExt = "*.bmp";
  ofn.Flags = OFN_PATHMUSTEXIST | OFN_OVERWRITEPROMPT;
  ofn.FlagsEx = OFN_EX_NOPLACESBAR;
  BOOL bRet;
  if(bSave)  bRet = GetSaveFileName(&ofn);
  else  bRet = GetOpenFileName(&ofn);
  if(bRet)
  return FALSE;
  strcpy(szPath, pszTempPath);
  delete [] pszTempPath;
  return TRUE;
}

---

What if the blue code is true? You'll return a value before delete[]ing pszTempPath. That's probably a memory leak . . . [edit] Beaten again. [/edit]

Also, if the blue code is true, then szPath is not touched. Is szPath initialized somewhere else? Because if it isn't, and you expect szPath to be initialized after calling this function, you might get a surprise . . .

Quote:

---

Originally Posted by dwks

Is szPath initialized somewhere else?

---

Code:

---

  ZeroMemory(szPath, MAX_PATH);

---

don't see where it's declared though.

Didn't see that. It's probably a global char array. And hopefully contains at least MAX_PATH elements.

Okay, all ready fixed those. Look, I KNOW my char arrays arn't the problem, I've had this with many of my programs before.

Quote:

---

Originally Posted by Yarin

After searching this board and google I find out that this is a common problem, however, those people always fix their problems by fixing a buffer overflow problem. I'm smart enough to avoid that from the beggining. So please help! Why isn't this working more than once??

---

Quote:

---

Originally Posted by Yarin

Okay, all ready fixed those. Look, I KNOW my char arrays arn't the problem, I've had this with many of my programs before.

---

Bwahahahahahahaha. :D

Make sure the lpstrFile member of the OPENFILENAME struct is null-terminated.

While not specific to your problem, if you already know the required size of your 'pszTempPath' variable is MAX_PATH then there's no need to use dynamic memory allocation.

ie.

Code:

---

char pszTempPath[MAX_PATH]="\0";

---

just a note:

Quote:

---

char pszTempPath[MAX_PATH]="\0";

---

is absolutely necessary for GetOpenFileName to function properly. from experience I know that both GetOpenFileName and GetSaveFileName seem to test the string, and will fail if it is not initialized to null. it doesn't apply to your problem however, as I see you are zeroing the pszTempPath memory block.

but shouldn't this:

Code:

---

if(bRet)
{
  delete [] pszTempPath;
  return FALSE;
}

---

be:

Code:

---

if(!bRet)
{
  delete [] pszTempPath;
  return FALSE;
}

---

Yes, I fixed that in the original code.

Here, I'll just down what I have now

Code:

---

char szPath[MAX_PATH];

...

int DoFileDialog(HWND hwnd, BOOL bSave)
{
  OPENFILENAME ofn;
  ZeroMemory(szPath, MAX_PATH);
  ZeroMemory(&ofn, sizeof(ofn));
  ofn.lStructSize = sizeof(ofn);
  ofn.hwndOwner = hwnd;
  ofn.lpstrFilter = "Bitmap Files\0*.bmp\0All Files\0*.*\0\0";
  ofn.lpstrFile = szPath;
  ofn.nMaxFile = MAX_PATH;
  ofn.lpstrDefExt = "*.bmp";
  ofn.Flags = OFN_PATHMUSTEXIST | OFN_OVERWRITEPROMPT;
  ofn.FlagsEx = OFN_EX_NOPLACESBAR;
  BOOL bRet;
  if(bSave)  bRet = GetSaveFileName(&ofn);
  else  bRet = GetOpenFileName(&ofn);
  if(!bRet)  return FALSE;
  return TRUE;
}

---

as you can see allocations are no longer the problem here (I think).
Like I said up top, it works the first time, and will only crash the 2nd+ time if I hover over somthing long enough for the little yellow details box to come up.

there are only 2 other things that I can think of.

1. it may be your ofn.Flags field. if you're opening a file, OFN_OVERWRITEPROMPT doesn't really make sense. I'd say try something like:

Code:

---

if(bSave)
  ofn.Flags = OFN_EXPLORER | OFN_PATHMUSTEXIST | OFN_OVERWRITEPROMPT;
else
  ofn.Flags = OFN_EXPLORER | OFN_FILEMUSTEXIST;

---

2. the little yellow details box you're talking about might have something to do with the OFN_EXPLORER flag (Explorer has the same feature, if you're talking about hovering the cursor over a file to view brief details). note that I added it in the above example, it doesn't hurt to give it a try.

otherwise I don't know what it could be.

Have you tried this code in a minimal program, which essentially exists only to call this function, and nothing else?

Does it work as expected ?

If so, then there is no problem with this particular function, and the problem is really elsewhere in your code.

Remember, if you're trashing memory (despite your protests), the effect of "not working" is often far removed from the true cause of the problem.

Another thing to consider is that Get[Save/Open]FileName are re-entrant. Your window procedure(s) will continue to be called while waiting for them to return. Consider what will happen if you are using static or global variables in your window procedure(s) and check WM_PAINT carefully, as it is likely to be called when the details tool tip disappears.

You where right anonytmouse. :) I set ofn.hwndOwner to null to seperate it from my main window, and the problem is no more.

Thanks for the help. :)

So what would I have to do to prevent that, while keeping my hwnd the parent?

Changing

Code:

---

      case WM_PAINT:
      {
        BeginPaint(hwnd, &ps);
        BitBlt(ps.hdc, 0, 0, nWndWide, nWndHigh, hdcBuffer, 0, 0, SRCCOPY);
        EndPaint(hwnd, &ps);
        break;
      }

---

to

Code:

---

      case WM_PAINT:
      {
        BeginPaint(hwnd, &ps);
        BitBlt(ps.hdc, 0, 0, nWndWide, nWndHigh, hdcBuffer, 0, 0, SRCCOPY);
        EndPaint(hwnd, &ps);
        return 0;
      }

---

works like I guessed, but is this a bad idea to do?

From MSDN:

Quote:

---

Originally Posted by MSDN

An application returns zero if it processes this message.

---

With "break" you are returning whatever value is in the return statement at the end of your window proc, whereas with "return 0" you are explicitly returning 0 right then and there. Since WM_PAINT says to do this, I'd go with the second one, but really, I believe your window proc should be returning 0 anyways, so if its not, thats another problem you might want to look into.