[PATCH 2/4] comdlg32: Add stub implementation of the Common Item dialogs.

Nikolay Sivov bunglehead at gmail.com
Tue Mar 22 15:40:26 CDT 2011


On 3/22/2011 23:24, David Hedberg wrote:
> On Mon, Mar 21, 2011 at 14:55, Nikolay Sivov<bunglehead at gmail.com>  wrote:
>
>>> +typedef struct FileDialogImpl {
>>> +    IFileDialog2 IFileDialog2_iface;
>>> +    IFileOpenDialog IFileOpenDialog_iface;
>>> +    IFileSaveDialog IFileSaveDialog_iface;
>>> +    LONG ref;
>>> +} FileDialogImpl;
>> Maybe it's better to use a union and enum type field here, if it can't be
>> open and save at the same time.
> At the moment I'm checking the vtbl pointers to determine which type
> of dialog it is in QueryInterface, although this looked better using
> the old COM coding style (no explicit check necessary).
>
> Is using a union and an explicit type field preferred?
I think it is, it makes it clear that dialogue instance is about save or 
open, not both.

By the way, IFileSaveDialog is based on IFileDialog, and you add 
IFileDialog2 too. It's a bit messy,
does target class object that will use FileDialogImpl really respond to 
IFileDialog2?

>>> +
>>> +    fdimpl = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
>>> sizeof(FileDialogImpl));
>>> +    if(!fdimpl)
>>> +        return E_OUTOFMEMORY;
>> Zero initialization is useless here.
> As mentioned above it's not completely useless currently, but I can
> initialize them explicitly.
Sure, that's what I meant. No need to initialize whole struct if you 
need only 'ref' to be 0 before QueryInterface.
> Thanks for the review.




More information about the wine-devel mailing list