how to save file with GetSaveFileName in win32?

Question

I write this code to get fileName to save my file :

#include "stdafx.h"
#include <windows.h>


int _tmain(int argc, _TCHAR* argv[])
{            
    OPENFILENAME ofn;

    char szFileName[MAX_PATH] = "";

    ZeroMemory(&ofn, sizeof(ofn));

    ofn.lStructSize = sizeof(ofn); 
    ofn.hwndOwner = NULL;
    ofn.lpstrFilter = (LPCWSTR)L"Text Files (*.txt)\0*.txt\0All Files (*.*)\0*.*\0";
    ofn.lpstrFile = (LPWSTR)szFileName;
    ofn.nMaxFile = MAX_PATH;
    ofn.Flags = OFN_EXPLORER | OFN_FILEMUSTEXIST | OFN_HIDEREADONLY;
    ofn.lpstrDefExt = (LPCWSTR)L"txt";

    GetSaveFileName(&ofn);
    printf("the path is : %s\n", ofn.lpstrFile);
    getchar();
    return 0;
}

But the output is :

 the path is : H 

why ? Am I doing something wrong ?
I'm using Visual Studio 2008 on Windows 7.

Gaulish

+1 for complete example & well-formulated question. – Latinalatinate

Answer 1

This line:

printf("the path is : %s\n", ofn.lpstrFile);

should use the wide char version of printf.

wprintf(L"the path is : %s\n", ofn.lpstrFile);

Chalmer

Since the rest of the code, including the lpstrFile value, is based on _TCHAR, you should use _tprintf instead: _tprintf(_T("the path is : %s\n"), ofn.lpstrFile); – Korte

-1 This code is still dangerous because the buffer is too small. – Salome

Answer 2

The root problem is in these lines:

char szFileName[MAX_PATH] = "";
...
ofn.lpstrFile = (LPWSTR)szFileName;
ofn.nMaxFile = MAX_PATH;

This creates a buffer of MAX_PATH characters, but it tells the GetSaveFileName function that it's a buffer of MAX_PATH wide characters. This is likely to crash (or silently trample memory) when someone chooses a long path name.

The giveaway is the cast. Don't lie to the compiler or the libraries. They don't like that, and they'll always get their revenge in the end. Replace those lines with this:

WCHAR szFileName[MAX_PATH] = L"";
...
ofn.lpstrFile = szFileName;  // no cast needed
ofn.nMaxFile = MAX_PATH;

Now the selected filename will be returned as a string of wide characters. Tony The Lion's answer is correct in that that you need to use wprintf rather than printf to print strings of wide characters:

wprintf(L"the path is : %s\n", ofn.lpstrFile);  // though I'd use szFileName at this point

If you need the string in 8-bit characters instead of wide characters, you can use WideCharToMultiByte. But I would just stick with the wide character APIs in general.

Never cast unless you know exactly what it does and why it's necessary in your particular case.

Salome

Answer 3

You're both wrong, it's a simple C pointer/stack issue.

// WRONG:
char szFileName[MAX_PATH] = "";

This confuses arrays and pointers, you declare an array on the stack, but then change its memory address to point to an empty string in the data section. In other words, a buffer overflow.

// RIGHT:
char szFileName[MAX_PATH];
ZeroMemory(szFileName, MAX_PATH);

This declares a character array on the stack and initializes all elements to null terminator.

Hope that helps!

Rockabilly

Literal strings on the right side of array initializers have special meaning. The statement char szFileName[MAX_PATH] = ""; does not create a pointer. It initializes szFileName[0] to '\0', i.e. the null terminator. – Merrell