Hmm ... I guess OpenPrinter doesn't fail often.
More of a concern is that I don't see where we (before or after this
change) call ClosePrinter
in the case of success.
We certainly don't want to close it here because acc to
*https://docs.microsoft.com/en-us/windows/win32/printdocs/findfirstprinterchangenotification*
Callers of FindFirstPrinterChangeNotification must ensure that the
printer handle passed into
FindFirstPrinterChangeNotification remains valid until
FindClosePrinterChangeNotification is called.
If the printer handle is closed before the printer change notification
handle, further notification
will fail to be delivered.
Are we leaking the handle ? Looks that way to me ..
-phil
On 3/31/20, 3:21 PM, 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
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.
