Looks good, Matthias. ..Thomas
On Wed, Jun 20, 2018 at 2:39 PM, Baesken, Matthias <matthias.baes...@sap.com> wrote: > Hi Thomas and Alan, thanks for the reviews. > > I adjusted the expression like Thomas suggested and adjusted the file > copyright year too : > > http://cr.openjdk.java.net/~mbaesken/webrevs/8205416.1/ > > > Best regards, Matthias > > > >> -----Original Message----- >> From: Thomas Stüfe [mailto:thomas.stu...@gmail.com] >> Sent: Mittwoch, 20. Juni 2018 14:17 >> To: Baesken, Matthias <matthias.baes...@sap.com> >> Cc: core-libs-dev@openjdk.java.net >> Subject: Re: RFR [XS] : 8205416 : windows: fix checking of CloseHandle return >> code in Java_java_io_FileCleanable_cleanupClose0 >> >> Good catch. >> >> Could you please change the expression to either >> >> CloseHandle() == FALSE (uppercase) >> >> or >> >> !CloseHandle(). >> >> ? which would be the standard windows way of writing this. >> >> I am curious whether we now get a bunch of follow up errors, since >> before we never catched a failing CloseHandle. A typical reason why >> this could fail would be a double close. >> >> ..Thomas >> >> >> On Wed, Jun 20, 2018 at 2:09 PM, Baesken, Matthias >> <matthias.baes...@sap.com> wrote: >> > Please review this small fix for a return code handling of windows function >> CloseHandle . >> > >> > MSDN documents CloseHandle here : https://msdn.microsoft.com/de- >> de/library/windows/desktop/ms724211(v=vs.85).aspx >> > .... >> > Return value >> > If the function succeeds, the return value is nonzero. >> > If the function fails, the return value is zero. To get extended error >> information, call GetLastError<https://msdn.microsoft.com/de- >> de/library/windows/desktop/ms679360(v=vs.85).aspx>. >> > >> > >> > However until this patch, Java_java_io_FileCleanable_cleanupClose0 >> > Checked for return code -1 of CloseHandle in error cases . >> > >> > >> > >> > >> > Bug: >> > >> > https://bugs.openjdk.java.net/browse/JDK-8205416 >> > >> > >> > Webrev : >> > >> > http://cr.openjdk.java.net/~mbaesken/webrevs/8205416/ >> > >> > >> > >> > Thanks, Matthias
