From 25b945ad24d824abedf9304ff09b98b6ec350d5d Mon Sep 17 00:00:00 2001
From: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>
Date: Wed, 4 Dec 2024 14:40:27 -0500
Subject: [PATCH] Review updates

---
 .../share/classes/java/lang/System.java       |  4 +-
 .../java/security/AccessController.java       | 60 ++++++++++---------
 runtime/jcl/common/java_lang_Class.cpp        |  6 +-
 .../DataHelperTests/DataHelperTests.xml       |  6 +-
 .../DataHelperTests_SecurityManager.xml       | 36 +++++------
 5 files changed, 58 insertions(+), 54 deletions(-)

diff --git a/jcl/src/java.base/share/classes/java/lang/System.java b/jcl/src/java.base/share/classes/java/lang/System.java
index cd3139854f6..56590c938e1 100644
--- a/jcl/src/java.base/share/classes/java/lang/System.java
+++ b/jcl/src/java.base/share/classes/java/lang/System.java
@@ -1350,7 +1350,7 @@ public static void setSecurityManager(final SecurityManager s) {
 /*[IF JAVA_SPEC_VERSION >= 24]*/
 	/*[MSG "K0B03", "Setting a Security Manager is not supported"]*/
 	throw new UnsupportedOperationException(Msg.getString("K0B03")); //$NON-NLS-1$
-/*[ELSE] JAVA_SPEC_VERSION >= 24*/
+/*[ELSE] JAVA_SPEC_VERSION >= 24 */
 /*[IF CRIU_SUPPORT]*/
 	if (openj9.internal.criu.InternalCRIUSupport.isCRIUSupportEnabled()) {
 		/*[MSG "K0B02", "Enabling a SecurityManager currently unsupported when -XX:+EnableCRIUSupport is specified"]*/
@@ -1428,7 +1428,7 @@ public Void run() {
 		currentSecurity.checkPermission(com.ibm.oti.util.RuntimePermissions.permissionSetSecurityManager);
 	}
 	security = s;
-/*[ENDIF] JAVA_SPEC_VERSION >= 24*/
+/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
 }
 
 /**
diff --git a/jcl/src/java.base/share/classes/java/security/AccessController.java b/jcl/src/java.base/share/classes/java/security/AccessController.java
index af767199ed5..f1bbeb38da7 100644
--- a/jcl/src/java.base/share/classes/java/security/AccessController.java
+++ b/jcl/src/java.base/share/classes/java/security/AccessController.java
@@ -47,7 +47,7 @@
 /*[ENDIF] JAVA_SPEC_VERSION >= 17 */
 public final class AccessController {
 /*[IF JAVA_SPEC_VERSION >= 24]*/
-	private static AccessControlContext ACC_NO_PERM = new AccessControlContext(
+	private static final AccessControlContext ACC_NO_PERM = new AccessControlContext(
 			new ProtectionDomain[] { new ProtectionDomain(null, null) });
 /*[ELSE] JAVA_SPEC_VERSION >= 24 */
 	static {
@@ -654,7 +654,6 @@ private static int getNewAuthorizedState(AccessControlContext acc, ProtectionDom
 	}
 	return newAuthorizedState;
  }
-/*[ENDIF] JAVA_SPEC_VERSION < 24 */
 
 /**
  * Helper method to combine the ProtectionDomain objects
@@ -722,6 +721,7 @@ static ProtectionDomain[] toArrayOfProtectionDomains(Object[] domains, AccessCon
 
 	return answer;
 }
+/*[ENDIF] JAVA_SPEC_VERSION < 24 */
 
 /**
  * Performs the privileged action specified by <code>action</code>.
@@ -770,12 +770,15 @@ public static <T> T doPrivileged(PrivilegedAction<T> action) {
  */
 @CallerSensitive
 public static <T> T doPrivileged(PrivilegedAction<T> action, AccessControlContext context) {
+	/*[IF JAVA_SPEC_VERSION >= 24]*/
+	return action.run();
+	/*[ELSE] JAVA_SPEC_VERSION >= 24 */
 	T result = action.run();
-	/*[IF JAVA_SPEC_VERSION < 24]*/
 	/*[PR 108112] context is not kept alive*/
 	keepalive(context);
-	/*[ENDIF] JAVA_SPEC_VERSION < 24 */
 	return result;
+	/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
+	
 }
 
 /**
@@ -839,12 +842,14 @@ public static <T> T doPrivileged (PrivilegedExceptionAction<T> action, AccessCon
 	throws PrivilegedActionException
 {
 	try {
+		/*[IF JAVA_SPEC_VERSION >= 24]*/
+		return action.run();
+		/*[ELSE] JAVA_SPEC_VERSION >= 24 */
 		T result = action.run();
-		/*[IF JAVA_SPEC_VERSION < 24]*/
 		/*[PR 108112] context is not kept alive*/
 		keepalive(context);
-		/*[ENDIF] JAVA_SPEC_VERSION < 24 */
 		return result;
+		/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
 	} catch (RuntimeException ex) {
 		throw ex;
 	} catch (Exception ex) {
@@ -873,9 +878,9 @@ public static <T> T doPrivileged (PrivilegedExceptionAction<T> action, AccessCon
 public static <T> T doPrivilegedWithCombiner(PrivilegedAction<T> action) {
 /*[IF JAVA_SPEC_VERSION >= 24]*/
 	return doPrivileged(action, null);
-/*[ELSE] JAVA_SPEC_VERSION >= 24*/
+/*[ELSE] JAVA_SPEC_VERSION >= 24 */
 	return doPrivileged(action, doPrivilegedWithCombinerHelper(null));
-/*[ENDIF] JAVA_SPEC_VERSION >= 24*/
+/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
 }
 
 /**
@@ -904,9 +909,9 @@ public static <T> T doPrivilegedWithCombiner(PrivilegedExceptionAction<T> action
 {
 /*[IF JAVA_SPEC_VERSION >= 24]*/
 	return doPrivileged(action, null);
-/*[ELSE] JAVA_SPEC_VERSION >= 24*/
+/*[ELSE] JAVA_SPEC_VERSION >= 24 */
 	return doPrivileged(action, doPrivilegedWithCombinerHelper(null));
-/*[ENDIF] JAVA_SPEC_VERSION >= 24*/
+/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
 }
 
 /*[IF JAVA_SPEC_VERSION < 24]*/
@@ -954,15 +959,15 @@ private static void checkPermsNPE(Permission... perms) {
 public static <T> T doPrivileged(PrivilegedAction<T> action,
 		AccessControlContext context, Permission... perms)
 {
-/*[IF JAVA_SPEC_VERSION < 24]*/
-	checkPermsNPE(perms);
-/*[ENDIF] JAVA_SPEC_VERSION < 24 */
-	T result = action.run();
-/*[IF JAVA_SPEC_VERSION < 24]*/
-	keepalive(context);
-	keepalive(perms);
-/*[ENDIF] JAVA_SPEC_VERSION < 24 */
-	return result;
+	/*[IF JAVA_SPEC_VERSION >= 24] */ 
+	return action.run();
+	/*[ELSE] JAVA_SPEC_VERSION >= 24 */
+	checkPermsNPE(perms);  
+	T result = action.run();  
+	keepalive(context);  
+	keepalive(perms);  
+	return result;  
+	/*[ENDIF] JAVA_SPEC_VERSION >= 24 */ 
 }
 
 /**
@@ -991,12 +996,11 @@ public static <T> T doPrivilegedWithCombiner(PrivilegedAction<T> action,
 		AccessControlContext context, Permission... perms)
 {
 /*[IF JAVA_SPEC_VERSION >= 24]*/
-	return doPrivileged(action, context, perms); // 24 - perms?
-/*[ELSE] JAVA_SPEC_VERSION >= 24*/
+	return doPrivileged(action, context, perms);
+/*[ELSE] JAVA_SPEC_VERSION >= 24 */
 	checkPermsNPE(perms);
 	return doPrivileged(action, doPrivilegedWithCombinerHelper(context), perms);
-/*[ENDIF] JAVA_SPEC_VERSION >= 24*/
-
+/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
 }
 
 /**
@@ -1030,15 +1034,15 @@ public static <T> T doPrivileged(PrivilegedExceptionAction<T> action,
 	throws PrivilegedActionException
 {
 	try {
-/*[IF JAVA_SPEC_VERSION < 24]*/
+		/*[IF JAVA_SPEC_VERSION >= 24]*/
+		return action.run();
+		/*[ELSE] JAVA_SPEC_VERSION >= 24 */
 		checkPermsNPE(perms);
-/*[ENDIF] JAVA_SPEC_VERSION < 24 */
 		T result = action.run();
-/*[IF JAVA_SPEC_VERSION < 24]*/
 		keepalive(context);
 		keepalive(perms);
-/*[ENDIF] JAVA_SPEC_VERSION < 24 */
 		return result;
+		/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
 	} catch (RuntimeException ex) {
 		throw ex;
 	} catch (Exception ex) {
@@ -1102,6 +1106,6 @@ private static AccessControlContext doPrivilegedWithCombinerHelper(AccessControl
 	}
 	return fixedContext;
 }
-/*[ENDIF] JAVA_SPEC_VERSION < 24*/
+/*[ENDIF] JAVA_SPEC_VERSION < 24 */
 
 }
diff --git a/runtime/jcl/common/java_lang_Class.cpp b/runtime/jcl/common/java_lang_Class.cpp
index 440f0dde3af..839946d4c9b 100644
--- a/runtime/jcl/common/java_lang_Class.cpp
+++ b/runtime/jcl/common/java_lang_Class.cpp
@@ -44,7 +44,7 @@ typedef enum {
 	STATE_IMPLIED = 1
 } StackWalkingStates;
 
-#define 	STACK_WALK_STATE_MAGIC 		(void *)1
+#define STACK_WALK_STATE_MAGIC ((void *)1)
 
 #if JAVA_SPEC_VERSION < 24
 typedef enum {
@@ -54,8 +54,8 @@ typedef enum {
 	OBJS_ARRAY_SIZE = 3
 } ObjsArraySizeNindex;
 
-#define 	STACK_WALK_STATE_LIMITED_DOPRIVILEGED		(void *)2
-#define 	STACK_WALK_STATE_FULL_DOPRIVILEGED		(void *)3
+#define STACK_WALK_STATE_LIMITED_DOPRIVILEGED ((void *)2)
+#define STACK_WALK_STATE_FULL_DOPRIVILEGED ((void *)3)
 
 static UDATA isPrivilegedFrameIterator(J9VMThread * currentThread, J9StackWalkState * walkState);
 static UDATA isPrivilegedFrameIteratorGetAccSnapshot(J9VMThread * currentThread, J9StackWalkState * walkState);
diff --git a/test/functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests.xml b/test/functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests.xml
index 52ff13aadb3..0958998514f 100644
--- a/test/functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests.xml
+++ b/test/functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests.xml
@@ -54,13 +54,13 @@
 	<if testVariable="SCMODE" testValue="213" resultVariable="currentMode" resultValue="$mode213$"/>
 	<if testVariable="SCMODE" testValue="214" resultVariable="currentMode" resultValue="$mode214$"/>
 
-	<echo value=" "/>
+	<echo value=""/>
 	<echo value="#######################################################"/>
 	<echo value="Running tests with command line options: $currentMode$"/>
-	<echo value=" "/>
+	<echo value=""/>
 	<exec command="$JAVA_EXE$ -version" quiet="false"/>
 	
-	<variable name="BOOTCP" value=" "/>
+	<variable name="BOOTCP" value=""/>
 
 	<exec command="$JAVA_EXE$ -Xshareclasses:destroyAll" quiet="false"/>
 	
diff --git a/test/functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests_SecurityManager.xml b/test/functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests_SecurityManager.xml
index 8748810f68a..22edc7b6a39 100644
--- a/test/functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests_SecurityManager.xml
+++ b/test/functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests_SecurityManager.xml
@@ -1,25 +1,25 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
 
 <!--
-  Copyright IBM Corp. and others 2024
+Copyright IBM Corp. and others 2024
 
-  This program and the accompanying materials are made available under
-  the terms of the Eclipse Public License 2.0 which accompanies this
-  distribution and is available at https://www.eclipse.org/legal/epl-2.0/
-  or the Apache License, Version 2.0 which accompanies this distribution and
-  is available at https://www.apache.org/licenses/LICENSE-2.0.
+This program and the accompanying materials are made available under
+the terms of the Eclipse Public License 2.0 which accompanies this
+distribution and is available at https://www.eclipse.org/legal/epl-2.0/
+or the Apache License, Version 2.0 which accompanies this distribution and
+is available at https://www.apache.org/licenses/LICENSE-2.0.
 
-  This Source Code may also be made available under the following
-  Secondary Licenses when the conditions for such availability set
-  forth in the Eclipse Public License, v. 2.0 are satisfied: GNU
-  General Public License, version 2 with the GNU Classpath
-  Exception [1] and GNU General Public License, version 2 with the
-  OpenJDK Assembly Exception [2].
+This Source Code may also be made available under the following
+Secondary Licenses when the conditions for such availability set
+forth in the Eclipse Public License, v. 2.0 are satisfied: GNU
+General Public License, version 2 with the GNU Classpath
+Exception [1] and GNU General Public License, version 2 with the
+OpenJDK Assembly Exception [2].
 
-  [1] https://www.gnu.org/software/classpath/license.html
-  [2] https://openjdk.org/legal/assembly-exception.html
+[1] https://www.gnu.org/software/classpath/license.html
+[2] https://openjdk.org/legal/assembly-exception.html
 
-  SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 OR GPL-2.0-only WITH OpenJDK-assembly-exception-1.0
+SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 OR GPL-2.0-only WITH OpenJDK-assembly-exception-1.0
 -->
 
 <!DOCTYPE suite SYSTEM "cmdlinetester.dtd">
@@ -54,13 +54,13 @@
 	<if testVariable="SCMODE" testValue="213" resultVariable="currentMode" resultValue="$mode213$"/>
 	<if testVariable="SCMODE" testValue="214" resultVariable="currentMode" resultValue="$mode214$"/>
 
-	<echo value=" "/>
+	<echo value=""/>
 	<echo value="#######################################################"/>
 	<echo value="Running tests with command line options: $currentMode$"/>
-	<echo value=" "/>
+	<echo value=""/>
 	<exec command="$JAVA_EXE$ -version" quiet="false"/>
 
-	<variable name="BOOTCP" value=" "/>
+	<variable name="BOOTCP" value=""/>
 	<variable name="SECURITY_ON" value="-Djava.security.policy==java.good.policy -Djava.security.manager"/>
 	<variable name="SECURITY_ON_READONLY_BADPOLICY" value="-Djava.security.policy==java.bad.readonly.policy -Djava.security.manager"/>
 	<variable name="SECURITY_ON_WRITEONLY_BADPOLICY" value="-Djava.security.policy==java.bad.writeonly.policy -Djava.security.manager"/>