RFR(S): 8165433: Convert Test_linked_list to Gtest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Tue Sep 6 19:20:15 UTC 2016


David,

Thank you for reviewing the test!

On 06.09.2016 02:47, David Holmes wrote:
> Hi Kirill,
>
> Generally seems okay but:
>
> - why is there no include for allocation.hpp or else 
> allocation.inline.hpp when it is obviously used in the cpp file
"utilities/linkedlist.hpp" is included and it includes 
"memory/allocation.hpp"
Do you think I need to include allocation.hpp in the test too?

>
> - why do you not use NULL when checking pointers eg::
>
>  73   ASSERT_NE(i, (Integer*) 0) << "Should find it";
>
> Explicitly casting 0 to a pointer is not good form IIRC.
Fixed in new WebRev: 
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8165433/webrev.01/

Thank you.

Regards, Kirill

>
> Otherwise conversion seems fine.
>
> Thanks,
> David
>
>> Thank you.
>>
>> Regards, Kirill



More information about the hotspot-runtime-dev mailing list