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.