[PATCH 2/5] qmgr: Implement IBackgroundCopyJob::GetDescription()
Nikolay Sivov
nsivov at codeweavers.com
Mon Nov 25 01:24:55 CST 2013
On 11/25/2013 11:14, Dmitry Timoshkov wrote:
> 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.
It's not changed, helper is a new addition. I think helper purpose is
clear from its name,
argument names also help with that.
> Also
> moving the parameter checks from the callers to helper doesn't look
> justified IMO.
This helper is about to be used in more methods with similar purpose to
return a string.
More information about the wine-devel
mailing list