Re: [PATCH 1/4] shell32: Add ShellLinkObject stubs.

[Nikolay Sivov]

On 9/13/20 11:33 PM, Myah Caron wrote:
> +typedef struct {
> - IShellLinkDual2 IShellLinkDual2_iface;
> - LONG ref;
> -
> - FolderItemImpl *item;
> - IShellLinkW *shell_link;
> - IPersistFile *persist_file;
>
> +} ShellLinkObjectImpl;

It looks like you're only using shell_link.
> +static HRESULT ShellLinkObject_Constructor(FolderItemImpl *item, IShellLinkDual2 **link)

It's not used until 3/4. I think it's fine to have tests in first patch,
and merge this one with 3/4.
> - This->shell_link = NULL;
> - hr = CoCreateInstance(&CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER,
> -
>                      &IID_IShellLinkW, (LPVOID*)&This->shell_link);
> - if (hr != S_OK) goto free_this;
> -
> - This->persist_file = NULL;
> - IShellLinkW_QueryInterface(This->shell_link, &IID_IPersistFile,
> -
>                           (LPVOID*)&This->persist_file);
> - if (hr != S_OK) goto free_this;

This is probably missing assignment to hr. For checks FAILED() is
usually sufficient.
> -
> - hr = IPersistFile_Load(This->persist_file, item->path, STGM_READ);
> - if (hr != S_OK) goto free_this;
> -
> - IShellLinkDual2_AddRef(&This->IShellLinkDual2_iface);
> -
> - *link = (IShellLinkDual2 *)&This->IShellLinkDual2_iface;
> - return S_OK;

Why AddRef here?
> -
> - free_this:
> - heap_free(This);
> - return hr;

It's easy to avoid goto in this case.