thanks Alex, pushed. -- Igor
> On Jun 10, 2020, at 1:05 PM, Alex Menkov <alexey.men...@oracle.com> wrote: > > Hi Igor, > > LGTM. > > --alex > > On 06/10/2020 12:29, Igor Ignatyev wrote: >> Hi Alex, >> sure, here is the incremental diff, so now Error is thrown if bb arrays are >> empty -- http://cr.openjdk.java.net/~iignatyev//8183040/webrev.0-1 >> Thanks, >> -- Igor >>> On Jun 10, 2020, at 12:20 PM, Alex Menkov <alexey.men...@oracle.com> wrote: >>> >>> Hi Igor, >>> >>> On 06/09/2020 20:11, Igor Ignatyev wrote: >>>> Hi Alex, >>>> as far as I can see, the caller just rethrows IOException as >>>> RuntimeException, so I don't think throwing IndexOutOfBoundsException >>>> would be much different, albeit it will be a bit more cryptic. yet given >>>> the content of /proc/sys/kernel/yama/ptrace_scope and >>>> /sys/fs/selinux/booleans/deny_ptrace is part of linux kernel contract, I >>>> doubt we will encounter IIOOBE in any reasonable setups. however, if you >>>> want I can check the length of bb arrays at L#171 and L#190 and throw an >>>> Error w/ message suggesting that something went completely wrong. >>> >>> Yes, the test still fails in the case, but if I see >>> IndexOutOfBoundsException (or something similar) as a test failure reason, >>> my first thought that this is the test issue. >>> Could you please add the checks. >>> >>> --alex >>> >>>> -- Igor >>>>> On Jun 9, 2020, at 6:36 PM, Alex Menkov <alexey.men...@oracle.com> wrote: >>>>> >>>>> Hi Igor, >>>>> >>>>> In SATestUtils.java you do >>>>> >>>>> var bb = ... Files.readAllBytes(...) ... >>>>> and then use bb[0] >>>>> >>>>> if the file has 0 length, old code throws EOFException and new one will >>>>> throw IndexOutOfBoundsException. >>>>> And looks like the caller doesn't expect it (it catches IOException). >>>>> >>>>> --alex >>>>> >>>>> On 06/09/2020 16:47, Igor Ignatyev wrote: >>>>>> http://cr.openjdk.java.net/~iignatyev//8183040/webrev.00 >>>>>>> >>>>>>> 38 lines changed: 8 ins; 16 del; 14 mod; >>>>>> Hi all, >>>>>> could you please review this small clean up of testlibrary classes which >>>>>> updates j.t.lib.Platform and j.t.l.SA.SATestUtils (as it now contains >>>>>> the methods which 8183040 was about) to use NIO file API? >>>>>> testing: test/hotspot/jtreg/serviceability >>>>>> webrev: http://cr.openjdk.java.net/~iignatyev//8183040/webrev.00 >>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8183040 >>>>>> Thanks, >>>>>> -- Igor
