The change is good. Thank you!
regards,
Boris
On 20.05.2019 3:43, David Holmes wrote:
Hi Daniil,
cc: Boris and Erik J.
On 20/05/2019 7:12 am, Daniil Titov wrote:
Please review the change that fixes the failure of
sun/management/jmxremote/bootstrap JMX tests on Windows platform.
While running, these tests invoke revokeall.exe utility and this
utility hangs.
The problem here is that invokeall.exe goes into an endless loop
while iterating over Access Control Entries (ACE) for a given file if
it encounters at least one ACE with the type different from
ACCESS_ALLOWED_ACE_TYPE.
The change fixes this problem. It also removes revokeall.exe binary
from the repository and changes the makefile to get it built instead.
Tier1, tier2, tier3, jdk_svc, and sun/management/jmxremote/bootstrap
tests succeeded in Mach5.
Webrev: http://cr.openjdk.java.net/~dtitov/8214545
Bug: https://bugs.openjdk.java.net/browse/JDK-8214545
I knew this seemed very familiar ... Boris had a fix for this a few
weeks ago under JDK-8220581. Similar but not identical to yours - see
below. Though getting rid of the exe from the repo is a good idea
(thanks Erik!).
A few comments
test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
Pre-existing:
! REVOKEALL="$TESTNATIVEPATH/revokeall.exe"
if [ ! -f "$REVOKEALL" ] ; then
I would expect a -x test not -f.
---
test/jdk/sun/management/windows/README
The first copyright year should be 2004.
25 This directory contains the source and the binary version
Delete "and the binary version".
---
test/jdk/sun/management/windows/exerevokeall.c
Pre-existing:
31 * file - suitable for NT/2000/XP only.
Please delete everything after "file".
355 i++;
356 count--;
The count-- is obvious as it is the loop counter, but it is far from
clear to me that i++ is correct. I don't fully understand the logic but
i is only incremented under very specific conditions. If you rewrote the
code to avoid the use of the continue then i would not be modified
except where it currently is.
Thanks,
David
-----
Thanks!
--Daniil