TCHAR correct usage

Hello, I'm trying to get a Browse Directory dialog to update a text box with the path selected. I had it working fine in Unicode build but now using TCHAR - I see the variable contains the correct path, but the textbox only gets updated with a single weird character. Any help would be great, thanks.

setting the text checking WM_COMMAND vars-

Code:

---

SetWindowText(textBox2,&buttonPush(hWnd));

---

browse function when Browse button is pressed -

Code:

---

TCHAR& buttonPush(HWND hWnd) {
                BROWSEINFO bi;
  TCHAR szDir[MAX_PATH];
  LPITEMIDLIST pidl;
  LPMALLOC pMalloc;

  if (SUCCEEDED(SHGetMalloc(&pMalloc)))
  {
      ZeroMemory(&bi,sizeof(bi));
      bi.hwndOwner = hWnd;
      bi.pszDisplayName = 0;
      bi.pidlRoot = 0;
      bi.ulFlags = BIF_RETURNONLYFSDIRS | BIF_STATUSTEXT | BIF_USENEWUI | BIF_NONEWFOLDERBUTTON;
          bi.pszDisplayName = szDir; // Address of a buffer to receive the display name of the folder selected by the user
                bi.lpszTitle = _T("Select a folder "); // Title of the dialog

      bi.lpfn = BrowseCallbackProc;
        pidl = SHBrowseForFolder(&bi);
               
                if (pidl)
                {
                        // Folder selected in dialog
                        TCHAR szReturnedDir[_MAX_PATH];

                        BOOL bRet = ::SHGetPathFromIDList(pidl, szReturnedDir);
                        if(bRet) return *szReturnedDir; // szReturnedDir holds the correct selected dir
                        pMalloc->Free(pidl);
                }
                pMalloc->Release();
    }
  return *szDir;
}

---

Your TCHAR szDir[MAX_PATH] is a local variable within the buttonPush(...) function. It goes out of scope before the buttonPush(...) function returns.
So its content is no more defined.
Use some non-POD type for the path like std::string/wstring or MFC CString class. Or just pass the TCHAR* as a parameter of the buttonPush(...) function.

You seem to be returning a reference to a TCHAR (which is either a char or a WCHAR depending on UNICODE definition). This reference is either to the first char of szReturnedDir if everything OK or szDir (which hasn't been initialised in case SHGetMalloc fails !!) if there is a problem. In both cases these are auto variables located upon the stack which are local in scope to the function and don't exist once the function has been exited. Use of any return value from buttonPush is therefore invalid. Depending upon what your calling program is doing and the state of memory it may give the impression of working but it isn't. This is why you are seeing a weird character as you are displaying what ever happens to be currently at the memory location returned by buttonPush which has probably been reused. The quick and dirty solution is to make szDir and szReturnedDir static. Then your function always returns a correct reference to valid memory.

Incidentially, as you seem to want to return a string - why are you just returing a character (TCHAR&)????? Shouldn't you be returning a pointer to a TCHAR array (TCHAR*) - and in your code use return szReturnedDir or return szdir if these have been declared static?

Thanks for the help, making the var static works. I might switch it to a function argument if that's smarter though. Is it?

>> Is it?
Yes.
static essentially makes them global - so they have all the drawbacks of globals.

gg

The problem though of passing TCHAR* as a parameter of the function is that the function does not know how much memory has been allocated so its possible to get the old problem of buffer over-run! You could have TCHAR* & as a reference parameter in which case your function could free existing memory and then allocate sufficient required. The calling program though would then have the responsibility to free memory it hadn't allocated which again is not ideal. In situations like this I tend to use a string reference as a parameter. This would then be

Code:

---

BOOL buttonPush(HWND hWnd, string& folder) {
...
  if (bret) {
      folder = szReturnedDir;
      return TRUE;
    }
....
}

---

where buttonPush returns TRUE if succeeds and FALSE if an error occurred.

This also has the advantage that it would be easy for the calling program to test for error (which currently you don't seem to do!). The calling code then might look like

Code:

---

#include <string>
using namespace std;
...
string folder;

    if (buttonPush(hWnd, folder)) {
        SetWindowText(textBox2, folder.c_str());
    }

---

This does away with having to use static arrays (bad), passing memory pointers (not good) and allocating and freeing memory.

Ah ok thank you