[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