On Tue, 17 Dec 2024 17:04:10 GMT, Alexander Zuev <kiz...@openjdk.org> wrote:
>>> > Although `icon != null` check is not strictly required here due to `hIcon >>> > != 0` check, added it as sanity check and to be consistent with other >>> > locations in Win32ShellFolder2.java where makeIcon() is called (which is >>> > followed by a null check for the returned icon). >>> >>> I'd leave it out here. As @azuev-java [notes >>> above](https://github.com/openjdk/jdk/pull/22776/files#r1887863199), the >>> likely problem is `hIcon == 0` rather than `makeIcon` returns `null`. >> >> Now that I've thought about it more, you're right `icon != null` is also >> needed… if the icon bits can't be extracted which can also occur if *GDI >> resources are exhausted*. > > Then i would suggest doing both checks in both places. No need to even try to > extract icon bits for hIcon == 0 but if we failed to extract icon for the > correct handle we should handle it too. Yes, I did [suggest it](https://github.com/openjdk/jdk/pull/22776#discussion_r1888597821) too. On the other hand, `makeIcon` starts with this condition https://github.com/openjdk/jdk/blob/4f44cf6bf2423a57a841be817f348e3b1e88f0eb/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L1030-L1031 Anyway, not calling `makeIcon` when `hIcon == 0` is clearer. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22776#discussion_r1888911087
