Build changes look good.

/Erik

On 2019-05-19 17: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


Reply via email to