[PATCH 2/5] qmgr: Implement IBackgroundCopyJob::GetDescription()

Dmitry Timoshkov dmitry at baikal.ru
Mon Nov 25 01:14:34 CST 2013


Nikolay Sivov <nsivov at codeweavers.com> wrote:

> +static HRESULT return_strval(const WCHAR *str, WCHAR **ret)
> +{
> +    int len;
> +
> +    if (!ret) return E_INVALIDARG;
> +
> +    len = strlenW(str);
> +    *ret = CoTaskMemAlloc((len+1)*sizeof(WCHAR));
> +    if (!*ret) return E_OUTOFMEMORY;
> +
> +    if (len)
> +        strcpyW(*ret, str);
> +    else
> +        **ret = 0;
> +    return S_OK;
> +}
> +
>  static inline BackgroundCopyJobImpl *impl_from_IBackgroundCopyJob2(IBackgroundCopyJob2 *iface)
>  {
>      return CONTAINING_RECORD(iface, BackgroundCopyJobImpl, IBackgroundCopyJob2_iface);
> @@ -313,17 +330,10 @@ static HRESULT WINAPI BITS_IBackgroundCopyJob_GetDisplayName(
>      LPWSTR *pVal)
>  {
>      BackgroundCopyJobImpl *This = impl_from_IBackgroundCopyJob2(iface);
> -    int n;
>  
> -    if (!pVal)
> -        return E_INVALIDARG;
> +    TRACE("(%p)->(%p)\n", This, pVal);
>  
> -    n = (strlenW(This->displayName) + 1) * sizeof **pVal;
> -    *pVal = CoTaskMemAlloc(n);
> -    if (*pVal == NULL)
> -        return E_OUTOFMEMORY;
> -    memcpy(*pVal, This->displayName, n);
> -    return S_OK;
> +    return return_strval(This->displayName, pVal);
>  }

What is the reason of changing pVal type from LPWSTR* to WCHAR** in
the helper? Leaving would LPWSTR* make it more clear IMO what the helper
is supposed to do and would simplify dereferencing logic a bit. Also
moving the parameter checks from the callers to helper doesn't look
justified IMO.

-- 
Dmitry.



More information about the wine-devel mailing list