RFR(S): 8192927: os::dir_is_empty is incorrect on Windows
Calvin Cheung
calvin.cheung at oracle.com
Thu Jan 4 22:12:41 UTC 2018
Hi Serguei,
Thanks for your review.
Here's an updated webrev with your suggested simplifications:
http://cr.openjdk.java.net/~ccheung/8192927/webrev.03/
thanks,
Calvin
On 1/4/18, 12:48 PM, serguei.spitsyn at oracle.com wrote:
> Hi Calvin,
>
> http://cr.openjdk.java.net/~ccheung/8192927/webrev.02/src/hotspot/os/windows/os_windows.cpp.frames.html
>
> I'd suggest a minor simplification.
> 4397 bool is_empty = false;
> Set the is_empty to 'true' instead.
> Then you can remove this alternative:
> 4427 if (f == INVALID_HANDLE_VALUE) {
> 4428 is_empty = true;
> It would look like this:
> if (f != INVALID_HANDLE_VALUE) {
> while (is_empty&& ::FindNextFileW(f,&fd)) {
> . . .
> Thanks,
> Serguei
>
>
> On 1/4/18 10:33, Calvin Cheung wrote:
>> Hi Ioi,
>>
>> Thanks for your review.
>>
>> I've made your suggested changes in the test case and ran it on 10
>> different windows hosts.
>>
>> updated webrev:
>> http://cr.openjdk.java.net/~ccheung/8192927/webrev.02/
>>
>> thanks,
>> Calvin
>>
>> On 1/3/18, 6:02 PM, Ioi Lam wrote:
>>> Hi Calvin,
>>>
>>> The code changes look good. I would suggesting running the tests in
>>> our test harness several times so you can run on different windows
>>> hosts.
>>>
>>>
>>> For the test case, I think there are too many use of "DoesntMatter"
>>> which may be confusing. I would suggest adding
>>>
>>> String classList[] = {"java/lang/Object"};
>>>
>>> and replacingall 'TestCommon.list("DoesntMatter")' with classList.
>>>
>>> Also,
>>>
>>> File subDir = new File(longDir, "DoesntMatter");
>>> ->
>>> File subDir = new File(longDir, "subdir");
>>>
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>>
>>> On 12/20/17 2:00 PM, Calvin Cheung wrote:
>>>> I've updated the change slightly:
>>>> - replaced the do-while loop with a while loop;
>>>> - free(search_path) should be os::free(search_path)
>>>>
>>>> updated webrev: http://cr.openjdk.java.net/~ccheung/8192927/webrev.01/
>>>>
>>>> thanks,
>>>> Calvin
>>>>
>>>> On 12/19/17, 5:01 PM, Calvin Cheung wrote:
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8192927
>>>>>
>>>>> This change is targeted for JDK 11.
>>>>> Please refer to the comment in the bug for a description of the
>>>>> change. It also handles path longer than MAX_PATH.
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8192927/webrev.00/
>>>>>
>>>>> Testing:
>>>>> hs-tier1, hs-tier2 on linux-x64, windows-x64, macosx
>>>>> appcds tests on the above platforms + sparcv9.
>>>>>
>>>>> thanks,
>>>>> Calvin
>>>
>
More information about the hotspot-runtime-dev
mailing list