On Mon, 23 Dec 2024 20:31:13 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>> Win32ShellFolder2.getSystemIcon() can result in NPE if icon is null. Sanity
>> null checks have been added.
>>
>>
>> long hIcon = getSystemIcon(iconType.getIconID()); //Can return null
>> Image icon = makeIcon(hIcon); // returns null if the condition (hIcon != 0L
>> && hIcon != -1L) is false
>> if (LARGE_ICON_SIZE != icon.getWidth(null)) { // NullPointerException --
>> icon is null
>>
>>
>> Occurs when Windows is unable to provide a system icon and the code assumes
>> an icon will **always** be returned, but the Windows API makes no such
>> guarantee.
>>
>> getSystemIcon calls LoadIcon (User32.dll). This can return null.
>> https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-loadicona
>
> Harshitha Onkar has updated the pull request incrementally with one
> additional commit since the last revision:
>
> hIcon check added
Marked as reviewed by aivanov (Reviewer).
src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1205:
> 1203: Image icon = null;
> 1204: long hIcon = getSystemIcon(iconType.getIconID());
> 1205: if (hIcon != 0) {
I suggest inverting the condition:
if (hIcon == 0) {
return null;
}
Image icon = makeIcon(hIcon);
The code has less indentation and has better readability, in my opinion.
-------------
PR Review: https://git.openjdk.org/jdk/pull/22776#pullrequestreview-2521912692
PR Review Comment: https://git.openjdk.org/jdk/pull/22776#discussion_r1896713653
