RFR(S): 8165433: Convert Test_linked_list to Gtest

David Holmes david.holmes at oracle.com
Mon Sep 5 23:47:52 UTC 2016


Hi Kirill,

On 6/09/2016 3:30 AM, Kirill Zhaldybin wrote:
> Dear all,
>
> Could you please review this fix for 8165433?
>
> The tests was separated into two -  for regular linked list and for
> sorted linked list, converted to Gtest, minor style changes implemented.

Minor style changes make the review harder as you have to scan for non 
cosmetic changes.

> WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8165433/webrev.00/
> CR: https://bugs.openjdk.java.net/browse/JDK-8165433

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

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

Otherwise conversion seems fine.

Thanks,
David

> Thank you.
>
> Regards, Kirill


More information about the hotspot-runtime-dev mailing list