On 01/04/2020 09:17, Prasanta Sadhukhan wrote:
On 01-Apr-20 3:51 AM, Sergey Bylokhov wrote:
Hi, Prasanta.
On 3/30/20 7:46 am, Prasanta Sadhukhan wrote:
I guess it is better if you directly pass NULL to OpenPrinter,
rather than create an unnecessary variable.
That was done only for documentation purposes.
Also, I think it will be good if we do a ClosePrinter() if
FindFirstPrinterChangeNotification() fails.
Nice catch! webrev is updated:
http://cr.openjdk.java.net/~serb/8241829/webrev.01
Looks good now. But since this is a cleanup exercise, I would have
liked to remove the unnecessary variable and still keep the
documentation, can't we (like "NULL in 1st argument indicates using
local printer server")? but anyway, not a dealbreaker, upto you.
I agree, I'd remove printerName local variable. You can modify the
original comment:
// If pPrinterName (the first parameter) is NULL, "it indicates
the local printer server" - MSDN.
I'd also align jobject parameter to JNIEnv as done in other functions.
However, it's nitpicking. :)
Otherwise, looks good to me.
Regards,
Alexey
Regards
Prasanta
Regards
Prasanta
On 30-Mar-20 7:13 PM, Sergey Bylokhov wrote:
Hello.
Please review the fix for jdk/client.
Bug: https://bugs.openjdk.java.net/browse/JDK-8241829
Fix: http://cr.openjdk.java.net/~serb/8241829/webrev.00
In PrintServiceLookupProvider.java we always pass NULL to
the notifyFirstPrinterChange as a name of the printer, so
we can remove the code which expects a non-null value.
