Re: [PATCH 1/2] gdi32: Initialize rgbReserved to zero in SetDIBColorTable

[Nikolay Sivov]

On 11/22/2016 06:36 PM, Daniel Lehman wrote:
> removes a TODO
> Signed-off-by: Daniel Lehman
>
> dlls/gdi32/dib.c          | 9 ++++++++-
>   dlls/gdi32/tests/bitmap.c | 3 ++-
>   2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/gdi32/dib.c b/dlls/gdi32/dib.c
> index 36a7b0f..6636f23 100644
> --- a/dlls/gdi32/dib.c
> +++ b/dlls/gdi32/dib.c
> @@ -922,7 +922,7 @@ INT WINAPI SetDIBitsToDevice(HDC hdc, INT xDest, INT yDest, DWORD cx,
>   UINT WINAPI SetDIBColorTable( HDC hdc, UINT startpos, UINT entries, const RGBQUAD *colors )
>   {
>       DC * dc;
> - UINT result = 0;
> -
>
> UINT i, result = 0;
>  BITMAPOBJ * bitmap;
>
> if (!(dc = get_dc_ptr( hdc ))) return 0;
>
> @@ -933,6 +933,13 @@ UINT WINAPI SetDIBColorTable( HDC hdc, UINT startpos, UINT entries, const RGBQUA
>           {
>               result = min( entries, bitmap->dib.dsBmih.biClrUsed - startpos );
>               memcpy(bitmap->color_table + startpos, colors, result * sizeof(RGBQUAD));
> -
>        for (i = 0; i < result; i++)
> -
>        {
> -
>            bitmap->color_table[startpos + i].rgbBlue     = colors[i].rgbBlue;
> -
>            bitmap->color_table[startpos + i].rgbGreen    = colors[i].rgbGreen;
> -
>            bitmap->color_table[startpos + i].rgbRed      = colors[i].rgbRed;
> -
>            bitmap->color_table[startpos + i].rgbReserved = 0;
> -
>        }
>     }

Doesn't this make memcpy() redundant? A couple of options that could be 
used:

- leave memcpy() and only reset rgbReserved() in the loop;
- remove memcpy();
- remove memcpy(), add another memcpy() for a size of three fields 
instead of a quad, keep separate rgbReserved assignment;

Not sure which one will be the fastest, could depend on 'result' value too.
>       GDI_ReleaseObj( dc->hBitmap );
>
> diff --git a/dlls/gdi32/tests/bitmap.c b/dlls/gdi32/tests/bitmap.c
> index 5539283..88250ca 100644
> --- a/dlls/gdi32/tests/bitmap.c
> +++ b/dlls/gdi32/tests/bitmap.c
> @@ -880,12 +880,13 @@ static void test_dibsections(void)
>       ok( ret == 1, "SetDIBColorTable returned unexpected result %u\n", ret );
>       ok( rgb[0].rgbReserved == 123, "Expected rgbReserved = 123, got %u\n", rgb[0].rgbReserved );
> - rgb[0].rgbRed = rgb[0].rgbGreen = rgb[0].rgbBlue = rgb[0].rgbReserved = -1;
>  ret = GetDIBColorTable( hdcmem, 0, 1, rgb );
>  ok( ret == 1, "GetDIBColorTable returned unexpected result %u\n", ret );
>  ok( rgb[0].rgbRed == 1, "Expected rgbRed = 1, got %u\n", rgb[0].rgbRed );
>  ok( rgb[0].rgbGreen == 2, "Expected rgbGreen = 2, got %u\n", rgb[0].rgbGreen );
>  ok( rgb[0].rgbBlue == 3, "Expected rgbBlue = 3, got %u\n", rgb[0].rgbBlue );
> - todo_wine ok( rgb[0].rgbReserved == 0, "Expected rgbReserved = 0, got %u\n", rgb[0].rgbReserved );
> -
>
> ok( rgb[0].rgbReserved == 0, "Expected rgbReserved = 0, got %u\n", rgb[0].rgbReserved );
>
> SelectObject(hdcmem, oldbm);
>  DeleteObject(hdib);