[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [PATCH 2/5] qmgr: Implement IBackgroundCopyJob::GetDescription()
From: Nikolay Sivov <nsivov () codeweavers ! com>
Date: 2013-11-25 7:24:55
Message-ID: 5292FB47.1020604 () codeweavers ! com
[Download RAW message or body]
On 11/25/2013 11:14, Dmitry Timoshkov wrote:
> Nikolay Sivov <nsivov@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.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic