Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suspicious definition of `BROWSEINFO` in Interop code #766

Open
hughbe opened this Issue Apr 13, 2019 · 5 comments

Comments

Projects
None yet
5 participants
@hughbe
Copy link
Contributor

hughbe commented Apr 13, 2019

In the C# code:

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Auto)]
public class BROWSEINFO
{
    public IntPtr hwndOwner; //HWND        hwndOwner;    // HWND of the owner for the dialog

    public int
        iImage; //int          iImage;                      // output var: where to return the Image index.

    public IntPtr
        lParam; //LPARAM       lParam;                      // extra info that's passed back in callbacks

    public IntPtr lpfn; //BFFCALLBACK  lpfn;            // Call back pointer

    public string lpszTitle; //LPCWSTR      lpszTitle;           // text to go in the banner over the tree.
    public IntPtr pidlRoot; //LPCITEMIDLIST pidlRoot;   // Root ITEMIDLIST

    // For interop purposes, send over a buffer of MAX_PATH size. 
    public IntPtr pszDisplayName; //LPWSTR       pszDisplayName;      // Return display name of item selected.
    public int ulFlags; //UINT         ulFlags;                     // Flags that control the return stuff
}

In the native code:

typedef struct _browseinfoA {
  HWND              hwndOwner;
  PCIDLIST_ABSOLUTE pidlRoot;
  LPSTR             pszDisplayName;
  LPCSTR            lpszTitle;
  UINT              ulFlags;
  BFFCALLBACK       lpfn;
  LPARAM            lParam;
  int               iImage;
} BROWSEINFOA, *PBROWSEINFOA, *LPBROWSEINFOA;

Note that the order differs

@weltkante

This comment has been minimized.

Copy link

weltkante commented Apr 13, 2019

Windows 98 was the last version where CharSet Auto was using the ANSI API, so you'll probably want to compare against BROWSEINFOW, but the order is probably the same anyways. Just mentioning it to avoid mistakes when reviewing the struct layout.

@danmosemsft

This comment has been minimized.

Copy link
Member

danmosemsft commented Apr 16, 2019

typedef struct _browseinfoW {
    HWND        hwndOwner;
    PCIDLIST_ABSOLUTE pidlRoot;
    LPWSTR       pszDisplayName;        // Return display name of item selected.
    LPCWSTR      lpszTitle;                     // text to go in the banner over the tree.
    UINT         ulFlags;                       // Flags that control the return stuff
    BFFCALLBACK  lpfn;
    LPARAM       lParam;                        // extra info that's passed back in callbacks
    int          iImage;                        // output var: where to return the Image index.
} BROWSEINFOW, *PBROWSEINFOW, *LPBROWSEINFOW;
@danmosemsft

This comment has been minimized.

Copy link
Member

danmosemsft commented Apr 17, 2019

@JeremyKuhne

This comment has been minimized.

Copy link
Member

JeremyKuhne commented Apr 17, 2019

This was fixed with #769. One nit- we should really replace CharSet.Auto with CharSet.Unicode for clarity when we get a chance. As @weltkante points out this was a 9x thing that is always Unicode on our supported Windows OSes for over a decade now. :)

@dreddy-work

This comment has been minimized.

Copy link
Member

dreddy-work commented Apr 19, 2019

@hughbe do you want to go ahead with Jeremy's suggetion and close the issue with PR?

@dreddy-work dreddy-work added this to the Future milestone Apr 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.