``` +/**************************************************************************** + * StockIconIdToResourceId [SHELL32.@] + * + * Map the SHSTOCKICONID value to shresdef.h IDI_ values + */ ```
Those "shell32.@" annotations are used for exports, which this isn't. This comment isn't adding anything at all and I would remove it.
``` +INT WINAPI StockIconIdToResourceId(SHSTOCKICONID id) ```
This should be static, should not be WINAPI, and can we please use a name that's more in line with the usual internal code style and doesn't look like like it's an API function? "stock_icon_id_to_resource_id" would be better.
``` +#define MISSING_ICON(x) case x : FIXME("Missing stock icon %s\n", #x); return -1 ```
Why bother with these? I don't think it's that important to have a string printed, we can always look it up later.
``` + default: FIXME("Undefined stock icon id\n"); return -1; ```
And then in the actual default case you don't print the ID, which is the one thing that's necessary...
``` - if (flags) - FIXME("flags 0x%x not implemented\n", flags); + if (flags & ~SHGSI_ICON) + FIXME("unhandled flags 0x%x\n", flags); ```
Why change the message?
``` - if (flags & SHGSI_ICON) - sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(sii->iIcon)); + if (flags & SHGSI_ICON) { + sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(StockIconIdToResourceId(id))); + } ```
Why add braces?