RFR(S): 8170936: Logging: LogFileOutput.invalid_file_test crashes when executed twice.

Thomas Stüfe thomas.stuefe at gmail.com
Tue Dec 20 13:13:55 UTC 2016

(also, I am not a (R)eviewer, so I guess this does not count as real review)

On Tue, Dec 20, 2016 at 2:13 PM, Thomas Stüfe <thomas.stuefe at gmail.com>

> Hi Kirill,
> thanks a lot for fixing this! This was an annoyance on Windows.
> Just small remarks:
> - RemoveDirectory will only work on empty directories. Recursively
> deleting non-empty directories seems to be more difficult. So could you
> please add a comment that this function only deletes empty directories and
> is not intended to work recursively? Or rename the function to
> delete_empty_directory()?
> - You do not have to output errno if RemoveDirectory failed, that number
> will be unrelated to the error. GetLastError should be enough.
> I also wonder why all those utilitiy functions are imeplemented in a
> header file? I think it would be better for a C file (can change
> implementation without triggering rebuild; also binary may be smaller, at
> least on slowdebug).
> Kind regards, Thomas
> On Tue, Dec 20, 2016 at 1:56 PM, Kirill Zhaldybin <
> kirill.zhaldybin at oracle.com> wrote:
>> Dear all,
>> Could you please review this fix for 8170936?
>> The issue was windows specific - remove() function failed to delete a
>> directory on that platform.
>> I added delete_directory function which uses winapi RemoveDirectory
>> function and also does GTest style check on its result.
>> I also changed TEST to TEST_VM on a few tests from the same file since
>> they used ResourceMark and crashed if they were run isolated.
>> WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8170936/web
>> rev.00/
>> CR: https://bugs.openjdk.java.net/browse/JDK-8170936
>> Thank you.
>> Regards, Kirill

More information about the hotspot-runtime-dev mailing list