Re: [PATCH 1/5] gameux: Add implementation of IGameStatistics::SetCategoryTitle

[Nikolay Sivov]

On 9/26/2010 16:59, Mariusz Pluciński wrote:
> ---
>  dlls/gameux/gamestatistics.c       |   50 
> ++++++++++++++++++++++++++++++++++-
>  dlls/gameux/tests/gamestatistics.c |   14 +++++-----
>  2 files changed, 55 insertions(+), 9 deletions(-)
> @@ -177,8 +201,30 @@ static HRESULT WINAPI GameStatisticsImpl_SetCategoryTitle(
>       WORD categoryIndex,
>       LPCWSTR title)
>   {
> -    FIXME("stub\n");
> -    return E_NOTIMPL;
> +    HRESULT hr = S_OK;
> +    DWORD dwLength;
> +    GameStatisticsImpl *This = impl_from_IGameStatistics(iface);
> +
> +    TRACE("(%p, %d, %s)\n", This, categoryIndex, debugstr_w(title));
> +
> +    if(!title || categoryIndex>= MAX_CATEGORIES)
> +        hr = E_INVALIDARG;
> Just return here and be done with it, this will save you hr 
> initialization and unnecessary if (SUCCEEDED(hr)) after that.
> +
> +    if(SUCCEEDED(hr))
> +    {
> +        dwLength = lstrlenW(title);
> +
> +        if(dwLength>  MAX_CATEGORY_LENGTH)
> +        {
> +            hr = S_FALSE;
> +            dwLength = MAX_CATEGORY_LENGTH;
> +        }
> Does it really copy with hr == S_FALSE ?
> +
> +        lstrcpynW(This->stats.categories[categoryIndex].sName,
> +                  title, dwLength+1);
> +    }
> +
> +    return hr;
>   }

[PATCH 1/5] gameux: Add implementation of IGameStatistics::SetCategoryTitle

[Mariusz Pluciński]

W dniu 26.09.2010 15:07, Nikolay Sivov pisze:
> On 9/26/2010 16:59, Mariusz Pluciński wrote:
> > ---
> > dlls/gameux/gamestatistics.c | 50 ++++++++++++++++++++++++++++++++++-
> > dlls/gameux/tests/gamestatistics.c | 14 +++++-----
> > 2 files changed, 55 insertions(+), 9 deletions(-)
> > @@ -177,8 +201,30 @@ static HRESULT WINAPI
> > GameStatisticsImpl_SetCategoryTitle(
> > WORD categoryIndex,
> > LPCWSTR title)
> > {
> > - FIXME("stub\n");
> > - return E_NOTIMPL;
> > + HRESULT hr = S_OK;
> > + DWORD dwLength;
> > + GameStatisticsImpl *This = impl_from_IGameStatistics(iface);
> > +
> > + TRACE("(%p, %d, %s)\n", This, categoryIndex, debugstr_w(title));
> > +
> > + if(!title || categoryIndex>= MAX_CATEGORIES)
> > + hr = E_INVALIDARG;
> > Just return here and be done with it, this will save you hr
> > initialization and unnecessary if (SUCCEEDED(hr)) after that.
> > You're right, I'll fix it.
> > +
> > + if(SUCCEEDED(hr))
> > + {
> > + dwLength = lstrlenW(title);
> > +
> > + if(dwLength> MAX_CATEGORY_LENGTH)
> > + {
> > + hr = S_FALSE;
> > + dwLength = MAX_CATEGORY_LENGTH;
> > + }
> > Does it really copy with hr == S_FALSE ?
> > Yes, this behavior is subject of one of tests.

-- 
Best regards
Mariusz Pluciński