> On 28 Jul 2016, at 12:40, Alan Bateman <alan.bate...@oracle.com> wrote: > > On 27/07/2016 12:47, Paul Sandoz wrote: > >> Hi, >> >> I made an embarrassing mistake in the fix for >> >> https://bugs.openjdk.java.net/browse/JDK-8151163 >> All Buffer implementations should leverage Unsafe unaligned accessors >> >> The offset calculation for Unsafe access was incorrect, it’s easy to get >> confused because for heap buffers the offset is relative to the array, and >> for direct buffers the address (which can update for slices/duplicates). >> Disturbingly all existing tests were passing both for core and hotspot when >> i pushed to hs. >> >> As a penance i wrote a combinatorial test for buffer views to navigate the >> twisty maze of heap/direct, aligned/unaligned, little/big endian for >> accessing binary data and views from the source buffer. >> >> Please review: >> >> >> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8162458-byte-buffer-view-offset-access-incorrect/webrev/ >> >> (This may be a duplicate of [1]). >> >> Test has been verified to fail with the existing code. Focused JPRT runs >> pass, but i will kick off core/hotspot runs later on today. >> > It is disturbing that it wasn't caught by the original tests. The new test > does appear to overlap with existing tests but I think that is okay.
Yes, i wanted to write a focused test that explicitly enumerates the test cases (and is easy to add more), otherwise it can be really hard to track down which combination of properties are failing. > One small request is to trim down the really long lines as they are annoying > when doing side-by-side diffs. > I split up lines over 110 characters or more. > In any case, the offsets in the updated code look right for me but tests is > the only way to catch issues. > Thanks, Paul.
