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

[Thomas Stüfe]

(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>
wrote:

> 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
>>
>
>