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:16 UTC 2016


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