GetSaveFilename

[Ellis Dee]

I have a program using API calls for the common dialog functions, but have run into a minor issue with one of my users who runs the program on Wine. (Linux)

Here's how I load up the structure that I send to the API:

Private Function GetStructure(pstrPath As String, pstrFile As String, pstrFilter As String, pstrDefaultExt As String, penFlags As FlagsEnum) As OPENFILENAME
    Const OFN_FILEMUSTEXIST = &H1000
    Const OFN_PATHMUSTEXIST = &H800
    Const OFN_HIDEREADONLY = &H4
    Const OFN_LONGNAMES = &H200000
    Const OFN_OVERWRITEPROMPT = &H2
    Const OF_WRITE = &H1
    Const MAX_PATH = 260

    With GetStructure
        .lStructSize = Len(GetStructure)
        .hwndOwner = Screen.ActiveForm.hwnd
        .hInstance = App.hInstance
        .lpstrFilter = Replace(pstrFilter, "|", Chr(0)) & Chr(0)
        .nMaxFile = MAX_PATH + 1
        .nMaxFileTitle = MAX_PATH + 1
        .lpstrFileTitle = Space(MAX_PATH)
        .lpstrInitialDir = pstrPath
        .lpstrDefExt = pstrDefaultExt
        Select Case penFlags
            Case feOpen
                .lpstrTitle = "Open"
                .lpstrFile = Space(MAX_PATH)
                .flags = OFN_FILEMUSTEXIST + OFN_HIDEREADONLY + OFN_LONGNAMES
            Case feSaveAs
                .lpstrTitle = "Save As"
                .lpstrFile = pstrFile & Space$(MAX_PATH - Len(pstrFile))
                .flags = OFN_PATHMUSTEXIST + OFN_HIDEREADONLY + OFN_LONGNAMES + OF_WRITE ' + OFN_OVERWRITEPROMPT
        End Select
    End With
End Function

Then I handle the return value as follows:

ShowSaveAsDialog = Left$(typFileName.lpstrFile, InStr(typFileName.lpstrFile, Chr$(0)) - 1)

This works great in Windows, but for the guy running Wine, he's getting the full 260 characters in his save dialog default filename. If he just clicks the Save button without doing anything, the program crashes with a stack dump termination.

My question:

Can I change the MAX_PATH values in the structure part to be the length of the string, or will that cause problems in Windows if the user chooses a longer filename (or path) than what I'm putting there as a default?

[Bonnie West]

Here's how I load up the structure that I send to the API:

Can you also please post the API and UDT declarations? Thank you.

... but for the guy running Wine, he's getting the full 260 characters in his save dialog default filename.

Try changing this line:

.lpstrFile = pstrFile & Space$(MAX_PATH - Len(pstrFile))

to this:

.lpstrFile = pstrFile & vbNullChar & Space$(MAX_PATH - Len(pstrFile) - 1)

BTW: the built-in constant vbNullChar is equivalent to what the slower function Chr(0) returns.

[LaVolpe]

... he's getting the full 260 characters in his save dialog default filename

You are setting that to 260 spaces, suggest 260 vbNullChar. I'm thinking your MAX_PATH+1 may be the issue here? The MSDN documentation says the length must include the null terminator. Same relationship exists between nMaxFile & lpstrFile. Even though you have 261 as the length & a string of 260 characters, it's fine since VB sends the 260 character string + a null terminator AFAIK (total of 261). However, Wine may not handle strings the same in their implementation? I have no idea how they treat string values (ANSI/Unicode) in relation to API calls and structures. Simple enough to test, change max length members to 260 vs 261.

Yes, nMaxFile can be much larger & may need to be if multi-selection is ever used.

... will that cause problems in Windows if the user chooses a longer filename (or path) than what I'm putting there as a default?

For the title, haven't tested the result, maybe it's just truncated? But for the returned file name, if nMaxFile is not large enough, then API fails (not crashes) with "buffer too small" error returned via the CommDlgExtendedError API

[fafalone]

Not too familiar with the issues at hand since I've never used WINE, but what strikes me is that you're filling the buffer with a space, then retreiving the result by looking for a null-char, which is not the same. If WINE isn't null-terminating names like Windows, that would return the whole buffer depending on how the error was handled (that could relate to the crash if he does nothing-- that would result in crashing since it would be left(.., -1) -- but if he does something the spaces are being counted, and whole buffer gets null-terminated). Try filling the buffer with String$(MAX_PATH, vbNullChar) instead.

[fafalone]

I didn't retype, only edited my post, so how'd it wind up new and duplicated? mods please delete this

[Ellis Dee]

Not too familiar with the issues at hand since I've never used WINE, but what strikes me is that you're filling the buffer with a space, then retreiving the result by looking for a null-char, which is not the same. If WINE isn't null-terminating names like Windows, that would return the whole buffer depending on how the error was handled (that could relate to the crash if he does nothing-- that would result in crashing since it would be left(.., -1) -- but if he does something the spaces are being counted, and whole buffer gets null-terminated). Try filling the buffer with String$(MAX_PATH, vbNullChar) instead.

The return value isn't a problem; if he manually clears the default filename and types in something, it works fine.

It's the pre-loading of the default save name that gets sent to the dialog that's an issue.

[Ellis Dee]

Can you also please post the API and UDT declarations?

Private Type OPENFILENAME
    lStructSize As Long
    hwndOwner As Long
    hInstance As Long
    lpstrFilter As String
    lpstrCustomFilter As String
    nMaxCustFilter As Long
    nFilterIndex As Long
    lpstrFile As String
    nMaxFile As Long
    lpstrFileTitle As String
    nMaxFileTitle As Long
    lpstrInitialDir As String
    lpstrTitle As String
    flags As Long
    nFileOffset As Integer
    nFileExtension As Integer
    lpstrDefExt As String
    lCustData As Long
    lpfnHook As Long
    lpTemplateName As String
End Type

Private Declare Function GetSaveFileName Lib "comdlg32.dll" Alias "GetSaveFileNameA" (pOpenfilename As OPENFILENAME) As Long

[fafalone]

..that's exactly what I was talking about. Sorry if my post wasn't clear enough, but I'm saying your default name would crash with a left(,-1) error, and when he changes it, the whole buffer is returned--- is this not the problem? If that's the problem I'm saying it's caused because your buffer is spaces instead of null-chars, and WINE may behave differently making this an issue.

It could also be the +1 issue LaVolpe was talking about, either way we're both recommending switching to vbNullChar instead of Space, you really should give that a try.

[Ellis Dee]

I'll try manually adding the null terminator at the end of my string before adding the extra padding, as per the recommendations, and mark as resolved if it works.

Not loving supporting multi-platform use. heh.

[fafalone]

If that doesn't work or you don't want to worry about it, you should really be using null buffers anyway. String$(length, 0)

[Ellis Dee]

Yeah, I think I agree. I'll try that first instead.

It'll take several days before I hear back whether or not it worked for the Wine guy.

[Ellis Dee]

BTW: the built-in constant vbNullChar is equivalent to what the slower function Chr(0) returns.

On a side note, this doesn't appear to be true. The debug windows shows:

?chr(0) = vbnullstring
False

Also, the way I use Chr(0) in the original code:

.lpstrFilter = Replace(pstrFilter, "|", Chr(0)) & Chr(0)

.lpstrFilter ends up the same length as pstrFilter, since Chr(0) has a Len() of 1. By contrast, vbNullstring translates into an empty string, meaning if I switch Chr(0) to vbNullstring the Replace() will just remove the "|"s instead of replacing them with Chr(0).

[Bonnie West]

On a side note, this doesn't appear to be true.

Of course they're not equal; vbNullString (empty string) is different from vbNullChar (Null character).



BTW, is there a reason for the OF_WRITE flag (of the OpenFile function) in the following line?

.flags = OFN_PATHMUSTEXIST + OFN_HIDEREADONLY + OFN_LONGNAMES + OF_WRITE ' + OFN_OVERWRITEPROMPT

Even though OF_WRITE has the same value (0x00000001) as the OFN_READONLY flag of the OPENFILENAME structure, it still seems to be ignored by Windows when showing the Explorer-style "Save As" dialog box.

[Ellis Dee]

Of course they're not equal; vbNullString (empty string) is different from vbNullChar (Null character).

Ha! Check out the reading comprehension on Ellis!

Nice tip, thanks. I'll switch to vbNullChar.

BTW, is there a reason for the OF_WRITE flag (of the OpenFile function) in the following line?

.flags = OFN_PATHMUSTEXIST + OFN_HIDEREADONLY + OFN_LONGNAMES + OF_WRITE ' + OFN_OVERWRITEPROMPT

Even though OF_WRITE has the same value (0x00000001) as the OFN_READONLY flag of the OPENFILENAME structure, it still seems to be ignored by Windows when showing the Explorer-style "Save As" dialog box.

No idea. I originally wrote/adapted this code the better part of a decade ago.

[Tanner_H]

One of the tricky things with Wine is that it sometimes adheres to the Microsoft/MSDN spec closer than Windows itself does. For security reasons, it's particularly strict about things like buffer overflows, and given the number of string buffers in the OPENFILENAME struct, there are many places that an off-by-1 error may occur.

Conversely, one of the great things about Wine is that you can see how they actually implement a function. Here's the source code for GetSaveFilenameA/W: http://source.winehq.org/source/dlls/comdlg32/filedlg.c

One thing I notice in their implementation is that GetSaveFilenameA just wraps GetSaveFilenameW. You can see this clearly in the GetFileDialog95A() function, which forcibly converts all ANSI strings to Unicode, then passes the result to the Unicode-capable GetFileName95 function.

To that end, I wonder if you might solve the problem just by switching to GetSaveFilenameW, as that would remove a bunch of the hoops that Wine jumps through.

[Bonnie West]

One thing I notice in their implementation is that GetSaveFilenameA just wraps GetSaveFilenameW. You can see this clearly in the GetFileDialog95A() function, which forcibly converts all ANSI strings to Unicode, then passes the result to the Unicode-capable GetFileName95 function.

They likely did it on purpose to improve compatibility with Windows.

Unicode and ANSI Functions

When Microsoft introduced Unicode support to Windows, it eased the transition by providing two parallel sets of APIs, one for ANSI strings and the other for Unicode strings. For example, there are two functions to set the text of a window's title bar:

• SetWindowTextA takes an ANSI string.
• SetWindowTextW takes a Unicode string.

Internally, the ANSI version translates the string to Unicode. The Windows headers also define a macro that resolves to the Unicode version when the preprocessor symbol UNICODE is defined or the ANSI version otherwise.

#ifdef UNICODE
#define SetWindowText  SetWindowTextW
#else
#define SetWindowText  SetWindowTextA
#endif

In MSDN, the function is documented under the name SetWindowText, even though that is really the macro name, not the actual function name.

New applications should always call the Unicode versions. Many world languages require Unicode. If you use ANSI strings, it will be impossible to localize your application. The ANSI versions are also less efficient, because the operating system must convert the ANSI strings to Unicode at run time. Depending on your preference, you can call the Unicode functions explicitly, such as SetWindowTextW, or use the macros. The example code on MSDN typically calls the macros, but the two forms are exactly equivalent. Most newer APIs in Windows have just a Unicode version, with no corresponding ANSI version.

[Ellis Dee]

The Wine guy reports no change in the dialog (mis)behavior after I switched it to null buffers.

Any other ideas?

[fafalone]

Try using GetSaveFileNameW directly.. although there are some differences that you could try with A...

        .nMaxFile = MAX_PATH + 1
        .nMaxFileTitle = MAX_PATH + 1

Did you try fixing the potential off-by-one error by removing the +1? I've seen two other very well done implementations besides mine and none of them are adding the 1.

I'm not sure where it might introduce a bug, but the way you set things up with the function as the structure... I've never seen that before. Just in case you might want to use a more standardized format... if you really needed to return the whole structure it could be a byref argument...

Karl E. Peterson's implementation:

Public Function GetSaveFileName(Filename As String, _
                           Optional FileTitle As String, _
                           Optional OverwritePrompt As Boolean = True, _
                           Optional Filter As String = "All (*.*)| *.*", _
                           Optional FilterIndex As Long = 1, _
                           Optional InitDir As String, _
                           Optional DlgTitle As String, _
                           Optional DefaultExt As String, _
                           Optional Owner As Long = -1, _
                           Optional Flags As Long) As Boolean

   Dim opfile As OpenFileNameType, s As String
   With opfile
      .lStructSize = Len(opfile)

      ' Add in specific flags and strip out non-VB flags
      .Flags = (-OverwritePrompt * OFN_OVERWRITEPROMPT) Or _
               OFN_HIDEREADONLY Or _
               (Flags And CLng(Not (OFN_ENABLEHOOK Or _
                                    OFN_ENABLETEMPLATE)))
      ' Owner can take handle of owning window
      If Owner <> -1 Then .hWndOwner = Owner
      ' InitDir can take initial directory string
      .lpstrInitialDir = StrPtr(InitDir)
      ' DefaultExt can take default extension
      .lpstrDefExt = StrPtr(DefaultExt)
      ' DlgTitle can take dialog box title
      .lpstrTitle = StrPtr(DlgTitle)

      ' Make new filter with bars (|) replacing nulls and double null at end
      Dim Ch As String, i As Integer
      For i = 1 To Len(Filter)
         Ch = Mid$(Filter, i, 1)
         If Ch = "|" Or Ch = ":" Then
            s = s & vbNullChar
         Else
            s = s & Ch
         End If
      Next
      ' Put double null at end
      s = s & vbNullChar & vbNullChar
      .lpstrFilter = StrPtr(s)
      .nFilterIndex = FilterIndex

      ' Pad file and file title buffers to maximum path
      s = Filename & String$(MAX_PATH - Len(Filename), 0)
      .lpstrFile = StrPtr(s)
      .nMaxFile = MAX_PATH
      s = FileTitle & String$(MAX_PATH - Len(FileTitle), 0)
      .lpstrFileTitle = StrPtr(FileTitle)
      .nMaxFileTitle = MAX_PATH
      ' All other fields zero

      If GetSaveFileNameW(opfile) Then
         GetSaveFileName = True
         Filename = PointerToStringW(.lpstrFile)
         FileTitle = PointerToStringW(.lpstrFileTitle)
         Flags = .Flags
         ' Return the filter index
         FilterIndex = .nFilterIndex
         ' Look up the filter the user selected and return that
         Filter = FilterLookup(.lpstrFilter, FilterIndex)
      Else
         GetSaveFileName = False
         Filename = sEmpty
         FileTitle = sEmpty
         Flags = 0
         FilterIndex = 0
         Filter = sEmpty
      End If
   End With
End Function

Personally... unless you're supporting XP, Get[Open|Save]FileName is deprecated anyway, it's much better and easier to use IFileDialog (checked, is supported in WINE). It shouldn't have any of those issues.

Dim isiRes As IShellItem
Dim lPtr As Long
Dim FileFilter() As COMDLG_FILTERSPEC
ReDim FileFilter(1)

FileFilter(0).pszName = "Image Files"
FileFilter(0).pszSpec = "*.jpg;*.gif;*.bmp"

FileFilter(1).pszName = "All Files"
FileFilter(1).pszSpec = "*.*"

Set fsd = New FileSaveDialog

With fsd
    .SetTitle "Simple Save"
    .SetFileTypes 2, VarPtr(FileFilter(0).pszName) '2 is the number of entries in the file filters, but the varptr must always be what it is here
    .SetDefaultExtension ".jpg"
    .SetFileName "DefaultSaveFile.jpg"
    .Show Me.hWnd
    
    .GetResult isiRes
    isiRes.GetDisplayName SIGDN_FILESYSPATH, lPtr
    Text1.Text = BStrFromLPWStr(lPtr, True)
    
End With
Set isiRes = Nothing
Set fsd = Nothing

See the link in my sig... all it requires is a typelib.

[Ellis Dee]

I'm not sure where it might introduce a bug, but the way you set things up with the function as the structure... I've never seen that before.

It's a bit unusual to With...End With a function name, but having a function return a type structure isn't particularly exotic.

Personally... unless you're supporting XP, Get[Open|Save]FileName is deprecated anyway, it's much better and easier to use IFileDialog (checked, is supported in WINE). It shouldn't have any of those issues.

I would like to support XP, yes.

Regarding using a TypeLib, one of my basic requirements for this project is to have no components or references of any kind (beyond the core references for all VB6 projects) so that end users can simply unzip the program and double-click the exe, no installation of any kind required for any version of Windows since XP.



I'll try removing the +1s if only to be consistent with others' code, but I suspect this will just stay as a known issue for my program. The guy running Wine can still make it work, it's just mildly irritating for him.

[Bonnie West]

Here's yet another GetSaveFileNameW implementation that you might want to check out.



EDIT

The hInstance member of the OPENFILENAME structure is used only if either the OFN_ENABLETEMPLATEHANDLE or OFN_ENABLETEMPLATE flags are specified, otherwise it is ignored. I'm not sure whether the same behavior is exhibited when running via Wine, but I think it would be best if you commented this line out:

'.hInstance = App.hInstance

[Ellis Dee]

The hInstance member of the OPENFILENAME structure is used only if either the OFN_ENABLETEMPLATEHANDLE or OFN_ENABLETEMPLATE flags are specified, otherwise it is ignored. I'm not sure whether the same behavior is exhibited when running via Wine, but I think it would be best if you commented this line out:

'.hInstance = App.hInstance

Certainly worth a shot, I'll give it a try.

I updated the program yesterday, and barring unexpected issues I don't plan to post another update for people to download for a couple weeks, so I won't be able to test much between now and then.

[fafalone]

So best odds of success,
-eliminate hinstance
-eliminate +1
-use GetSaveFileNameW

---
ps- for reference, a typelib is only required in the IDE; it gets compiled in your project and is not a separate file to distribute

[Ellis Dee]

ps- for reference, a typelib is only required in the IDE; it gets compiled in your project and is not a separate file to distribute

Ah, good to know.

[Ellis Dee]

Getting ready to publish the next version for download; just want to implement the recommendations from this thread first.

Try using GetSaveFileNameW directly.. although there are some differences that you could try with A...

        .nMaxFile = MAX_PATH + 1
        .nMaxFileTitle = MAX_PATH + 1

Did you try fixing the potential off-by-one error by removing the +1? I've seen two other very well done implementations besides mine and none of them are adding the 1.

Just now I removed the +1s and tested.

.nMaxFile = MAX_PATH
...doesn't work at all. Neither the Open or Save As dialogs (both use this same code) do anything without the +1; I never get to see any dialogs at all, meaning this is a critical failure. Putting the +1 back in, everything works fine on my machine.

nMaxFileTitle = MAX_PATH
...works fine, at least on my machine. Does that mean I can remove the +1 and be reasonably confident it will also work on Wine, do you think? I'm not super confident in doing this because losing the +1 on nMaxFile makes the dialogs not work at all, so I'd hate to make this change only find out that the dialogs just stop working on Wine and I have to publish a whole new version one day later.

EDIT: Commenting out the following line causes no problems, so I'll go ahead and remove it:
.hInstance = App.hInstance

EDIT 2: I don't actually know how to change to GetSaveFilenameW.

[LaVolpe]

The .hInstance member is used if you are loading dialog from a template (i.e., res file), else ignored.

Regarding W version...

Private Declare Function GetSaveFileNameW Lib "comdlg32.dll"  (ByVal pOpenfilename As Long) As Long

'sample call
Call GetSaveFileNameW(VarPtr(myOpenFileNameStructure))

By passing the structure via its pointer, you avoid VB trying to do ANSI conversion on the strings

[Ellis Dee]

Cool, thanks much, GetSaveFilenameW implemented.

EDIT: Will publish today or tonight, should hear back within a day or two if the issue is resolved.

[xiaoyao]

Try using GetSaveFileNameW directly.. although there are some differences that you could try with A...

        .nMaxFile = MAX_PATH + 1
        .nMaxFileTitle = MAX_PATH + 1

Did you try fixing the potential off-by-one error by removing the +1? I've seen two other very well done implementations besides mine and none of them are adding the 1.

I'm not sure where it might introduce a bug, but the way you set things up with the function as the structure... I've never seen that before. Just in case you might want to use a more standardized format... if you really needed to return the whole structure it could be a byref argument...

Karl E. Peterson's implementation:

Public Function GetSaveFileName(Filename As String, _
                           Optional FileTitle As String, _
                           Optional OverwritePrompt As Boolean = True, _
                           Optional Filter As String = "All (*.*)| *.*", _
                           Optional FilterIndex As Long = 1, _
                           Optional InitDir As String, _
                           Optional DlgTitle As String, _
                           Optional DefaultExt As String, _
                           Optional Owner As Long = -1, _
                           Optional Flags As Long) As Boolean

   Dim opfile As OpenFileNameType, s As String
   With opfile
      .lStructSize = Len(opfile)

      ' Add in specific flags and strip out non-VB flags
      .Flags = (-OverwritePrompt * OFN_OVERWRITEPROMPT) Or _
               OFN_HIDEREADONLY Or _
               (Flags And CLng(Not (OFN_ENABLEHOOK Or _
                                    OFN_ENABLETEMPLATE)))
      ' Owner can take handle of owning window
      If Owner <> -1 Then .hWndOwner = Owner
      ' InitDir can take initial directory string
      .lpstrInitialDir = StrPtr(InitDir)
      ' DefaultExt can take default extension
      .lpstrDefExt = StrPtr(DefaultExt)
      ' DlgTitle can take dialog box title
      .lpstrTitle = StrPtr(DlgTitle)

      ' Make new filter with bars (|) replacing nulls and double null at end
      Dim Ch As String, i As Integer
      For i = 1 To Len(Filter)
         Ch = Mid$(Filter, i, 1)
         If Ch = "|" Or Ch = ":" Then
            s = s & vbNullChar
         Else
            s = s & Ch
         End If
      Next
      ' Put double null at end
      s = s & vbNullChar & vbNullChar
      .lpstrFilter = StrPtr(s)
      .nFilterIndex = FilterIndex

      ' Pad file and file title buffers to maximum path
      s = Filename & String$(MAX_PATH - Len(Filename), 0)
      .lpstrFile = StrPtr(s)
      .nMaxFile = MAX_PATH
      s = FileTitle & String$(MAX_PATH - Len(FileTitle), 0)
      .lpstrFileTitle = StrPtr(FileTitle)
      .nMaxFileTitle = MAX_PATH
      ' All other fields zero

      If GetSaveFileNameW(opfile) Then
         GetSaveFileName = True
         Filename = PointerToStringW(.lpstrFile)
         FileTitle = PointerToStringW(.lpstrFileTitle)
         Flags = .Flags
         ' Return the filter index
         FilterIndex = .nFilterIndex
         ' Look up the filter the user selected and return that
         Filter = FilterLookup(.lpstrFilter, FilterIndex)
      Else
         GetSaveFileName = False
         Filename = sEmpty
         FileTitle = sEmpty
         Flags = 0
         FilterIndex = 0
         Filter = sEmpty
      End If
   End With
End Function

Personally... unless you're supporting XP, Get[Open|Save]FileName is deprecated anyway, it's much better and easier to use IFileDialog (checked, is supported in WINE). It shouldn't have any of those issues.

Dim isiRes As IShellItem
Dim lPtr As Long
Dim FileFilter() As COMDLG_FILTERSPEC
ReDim FileFilter(1)

FileFilter(0).pszName = "Image Files"
FileFilter(0).pszSpec = "*.jpg;*.gif;*.bmp"

FileFilter(1).pszName = "All Files"
FileFilter(1).pszSpec = "*.*"

Set fsd = New FileSaveDialog

With fsd
    .SetTitle "Simple Save"
    .SetFileTypes 2, VarPtr(FileFilter(0).pszName) '2 is the number of entries in the file filters, but the varptr must always be what it is here
    .SetDefaultExtension ".jpg"
    .SetFileName "DefaultSaveFile.jpg"
    .Show Me.hWnd
    
    .GetResult isiRes
    isiRes.GetDisplayName SIGDN_FILESYSPATH, lPtr
    Text1.Text = BStrFromLPWStr(lPtr, True)
    
End With
Set isiRes = Nothing
Set fsd = Nothing

See the link in my sig... all it requires is a typelib.

where is FilterLookup?

FilterIndex = .nFilterIndex
         ' Look up the filter the user selected and return that
         Filter = FilterLookup(.lpstrFilter, FilterIndex)