shdocvw(2/2): rudimentary implementation of CLSID_InternetShortcut (try 3)

[Alexandre Julliard]

"Damjan Jovanovic" <damjan.jov at gmail.com> writes:

> +static HRESULT strdupAtoW(LPCSTR in, LPWSTR *out)
> +{

The usual pattern is to return the new pointer, not an HRESULT. The only
possible error is E_OUTOFMEMORY anyway.

> +static HRESULT WINAPI UniformResourceLocatorA_SetUrl(IUniformResourceLocatorA *url, LPCSTR pcszURL, DWORD dwInFlags)
> +{
> +    HRESULT hr;
> +    LPWSTR wideURL;
> +    InternetShortcut *This = impl_from_IUniformResourceLocatorA(url);
> +    TRACE("(%p, %s, 0x%x)\n", url, debugstr_a(pcszURL), dwInFlags);
> +    hr = strdupAtoW(pcszURL, &wideURL);
> +    if (SUCCEEDED(hr))
> +    {
> +        hr = This->uniformResourceLocatorW.lpVtbl->SetURL(&This->uniformResourceLocatorW, wideURL, dwInFlags);

In general you shouldn't access lpVtbl. You should either use the COM
macros, or if you know the target function like in this case, call it
directly. Here it would be even better to not call anything and simply
store the pointer to avoid duplicating the string twice.

> +static HRESULT WINAPI UniformResourceLocatorW_SetUrl(IUniformResourceLocatorW *url, LPCWSTR pcszURL, DWORD dwInFlags)
> +{
> +    InternetShortcut *This = impl_from_IUniformResourceLocatorW(url);
> +    TRACE("(%p, %s, 0x%x)\n", url, debugstr_w(pcszURL), dwInFlags);
> +    if (dwInFlags != 0)
> +        FIXME("ignoring unsupported flags 0x%x\n", dwInFlags);
> +    This->isDirty = TRUE;
> +    HeapFree(GetProcessHeap(), 0, This->url);
> +    if (pcszURL == NULL)
> +        This->url = NULL;
> +    else
> +    {
> +        This->url = heap_strdupW(pcszURL);
> +        if (This->url == NULL)
> +            return E_OUTOFMEMORY;

You should preserve the old value on error.

-- 
Alexandre Julliard
julliard at winehq.org