On Wed, Nov 26, 2008 at 10:14 PM, Dmitry Timoshkov
<[EMAIL PROTECTED]> wrote:
> "Andrew Riedi" <[EMAIL PROTECTED]> wrote:
>
>> - icon_entry->xHotspot = 1;
>> - icon_entry->yHotspot = 1;
>> + icon_entry->xHotspot = 1; /* Color planes for .ico. */
>> + icon_entry->yHotspot = ICON_BPP; /* BPP for .ico. */
>
> ...
>>
>> ok(icon_info.xHotspot == 1, "xHotspot is %u.\n",
>> icon_info.xHotspot);
>> - ok(icon_info.yHotspot == 1, "yHotspot is %u.\n",
>> icon_info.yHotspot);
>> + ok(icon_info.yHotspot == 32, "yHotspot is %u.\n",
>> icon_info.yHotspot);
>
> It should be ICON_BPP instead of 32 for consistency.
>
>> + /* Test loading an icon as an icon. */
>> + SetLastError(0xdeadbeef);
>> + handle = LoadImageA(NULL, "icon.ico", IMAGE_ICON, 0, 0,
>> LR_LOADFROMFILE);
>> + ok(handle != NULL, "LoadImage() failed.\n");
>> + error = GetLastError();
>> + ok(error == 0 ||
>> + broken(error == 0xdeadbeef) || /* Win9x */
>> + broken(error == ERROR_BAD_PATHNAME), /* Win98, WinMe */
>> + "Last error: %u\n", error);
>
> There is no point in testing last error value if the API didn't fail.
>
>> + /* Test the icon information. */
>> + SetLastError(0xdeadbeef);
>> + ret = GetIconInfo(handle, &icon_info);
>> + ok(ret, "GetIconInfo() failed.\n");
>> + error = GetLastError();
>> + ok(error == 0xdeadbeef, "Last error: %u\n", error);
>
> Same here.
>
>> + if (ret)
>> + {
>> + ok(icon_info.fIcon == TRUE, "fIcon == FALSE.\n");
>> + ok(icon_info.xHotspot == 16, "xHotspot is %u.\n",
>> icon_info.xHotspot);
>> + ok(icon_info.yHotspot == 16, "yHotspot is %u.\n",
>> icon_info.yHotspot);
>> + ok(icon_info.hbmColor != NULL, "No hbmColor!\n");
>> + ok(icon_info.hbmMask != NULL, "No hbmMask!\n");
>> + }
>
> 'if (ret)' is useless if you don't expect a failure.
>
>> +
>> + /* Clean up. */
>> + SetLastError(0xdeadbeef);
>> + ret = DestroyIcon(handle);
>> + ok(ret, "DestroyIcon() failed.\n");
>> + error = GetLastError();
>> + ok(error == 0xdeadbeef, "Last error: %u\n", error);
>
> Same here.
>
>
> --
> Dmitry.
>
All great tips. I will rewrite this and clean up some of the existing
code in that file. As always, thanks for looking over my work,
Dmitry!
--
Andrew Riedi
