On Mon, 3 Nov 2025 05:31:34 GMT, Alexander Zvegintsev <[email protected]>
wrote:
>> Phil Race has updated the pull request with a new target base due to a merge
>> or a rebase. The pull request now contains four commits:
>>
>> - 8370637
>> - 8370637
>> - Merge
>> - 8370637
>
> src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java line 956:
>
>> 954: }
>> 955:
>> 956: protected synchronized void beginPath() {
>
> Perhaps this is a topic for a different PR, but the `protected` keyword is
> redundant here, as well as in many other places in the class, because the
> `WPrinterJob` is a final class.
yes, another PR I think
> src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 1589:
>
>> 1587: if ((HDC)dc != NULL) {
>> 1588: DeletePrintDC((HDC)dc);
>> 1589: }
>
> Suggestion:
>
> if ((HDC)dc != NULL) {
> DeletePrintDC((HDC)dc);
> }
>
>
> The closing brace is misaligned here and in all other cases.
I missed this comment. fixing.
> src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 1871:
>
>> 1869: TRY;
>> 1870:
>> 1871: if ((HDC)printDC == NULL) {
>
> There is no need to place this check inside the try-catch block.
I had to decide whether to put them outside or inside and it seemed safest to
do it inside as a pattern.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27984#discussion_r2492175191
PR Review Comment: https://git.openjdk.org/jdk/pull/27984#discussion_r2492176209
PR Review Comment: https://git.openjdk.org/jdk/pull/27984#discussion_r2492179626
