Address comments

Co-authored-by: Tobi Ajila <[email protected]>
Co-authored-by: Jason Feng <[email protected]>
Co-authored-by: Keith W. Campbell <[email protected]>
Signed-off-by: Amarpreet Singh <[email protected]>

Author: 4 people

closed/src/java.base/share/native/libjli/criuhelpers.c

@@ -37,16 +37,15 @@ static const char *
 getCommandLineOptionValue(const char *optionName, int argc, char **argv, int *error)
 {
     const char *value = NULL;
-    int i = 0;
+    int i = argc - 1;
     size_t optionNameLength = strlen(optionName);
-    jboolean optionNameFound = JNI_FALSE;
-    *error = 0;
-    for (i = 0; i < argc; i++) {
+    *error = -1;
+    for (i = argc - 1; i >= 0; i--) {
         const char *arg = argv[i];
         if (0 == strncmp(arg, optionName, optionNameLength)) {
             const char *equals = arg + optionNameLength;
             if (('=' == *equals) || ('\0' == *equals)) {
-                optionNameFound = JNI_TRUE;
+                *error = 0;
                 value = equals;
                 if ('=' == *value) {
                     value += 1;
@@ -58,9 +57,6 @@ getCommandLineOptionValue(const char *optionName, int argc, char **argv, int *er
             }
         }
     }
-    if (!optionNameFound) {
-        *error = -1;
-    }
     return value;
 }
 
@@ -185,7 +181,7 @@ restoreFromCheckpoint(const char *checkpointDirectory, int logLevel, jboolean un
 {
     int restoreStatus = 0;
     int length = -1;
-    char *logLevelOption = NULL;
+    char logLevelOption[4] = { 0 };
     char *logFileOption = NULL;
     int argc = 0;
     const char *argv[9] = { NULL };
@@ -195,49 +191,32 @@ restoreFromCheckpoint(const char *checkpointDirectory, int logLevel, jboolean un
     argv[argc++] = checkpointDirectory;
     length = snprintf(NULL, 0, "-v%d", logLevel);
     if (length < 0) {
-        char logLevelString[2] = { 0 };
-        sprintf(logLevelString, "%d", logLevel);
-        JLI_ReportErrorMessage("Failed to calculate the length of the command option, value=%s, format=%s.", logLevelString, "-v%d");
+        JLI_ReportErrorMessage("Failed to calculate the length of option '-v%d'.", logLevel);
         restoreStatus = -1;
         goto done;
     }
-    logLevelOption = (char *)JLI_MemAlloc(length + 1);
-    if (NULL == logLevelOption) {
-        char logLevelString[2] = { 0 };
-        sprintf(logLevelString, "%d", logLevel);
-        JLI_ReportErrorMessage("Failed to allocate memory for the command option, value=%s, format=%s.", logLevelString, "-v%d");
+    if (snprintf(logLevelOption, sizeof(logLevelOption), "-v%d", logLevel) < 0) {
+        JLI_ReportErrorMessage("Failed to format the log level option '-v%d'.", logLevel);
         restoreStatus = -1;
         goto done;
     }
-    if (snprintf(logLevelOption, length + 1, "-v%d", logLevel) < 0) {
-        char logLevelString[2] = { 0 };
-        sprintf(logLevelString, "%d", logLevel);
-        JLI_ReportErrorMessage("Failed to allocate memory for the command option, value=%s, format=%s.", logLevelString, "-v%d");
-        restoreStatus = -1;
-        goto freeLogLevelOption;
-    }
     argv[argc++] = logLevelOption;
     argv[argc++] = "--shell-job";
     if (unprivilegedModeOn) {
         argv[argc++] = "--unprivileged";
     }
     if (NULL != logFile) {
-        length = snprintf(NULL, 0, "--log-file=%s", logFile);
-        if (length < 0) {
-            JLI_ReportErrorMessage("Failed to calculate the length of the command option, value=%s, format=%s.", logFile, "--log-file=%s");
-            restoreStatus = -1;
-            goto freeLogLevelOption;
-        }
-        logFileOption = (char *)JLI_MemAlloc(length + 1);
+        length = strlen(logFile) + sizeof("--log-file=%s") - 1;
+        logFileOption = (char *)JLI_MemAlloc(length - 1);
         if (NULL == logFileOption) {
-            JLI_ReportErrorMessage("Failed to allocate memory for the command option, value=%s, format=%s.", logFile, "--log-file=%s");
+            JLI_ReportErrorMessage("Failed to allocate memory for option '--log-file=%s'.", logFile);
             restoreStatus = -1;
-            goto freeLogLevelOption;
+            goto done;
         }
-        if (snprintf(logFileOption, length + 1, "--log-file=%s", logFile) < 0) {
-            JLI_ReportErrorMessage("Failed to allocate memory for the command option, value=%s, format=%s.", logFile, "--log-file=%s");
+        if (snprintf(logFileOption, length - 1, "--log-file=%s", logFile) < 0) {
+            JLI_ReportErrorMessage("Failed to format option '--log-file=%s'.", logFile);
             restoreStatus = -1;
-            goto freeLogFileOption;
+            goto done;
         }
         argv[argc++] = logFileOption;
     }
@@ -246,15 +225,10 @@ restoreFromCheckpoint(const char *checkpointDirectory, int logLevel, jboolean un
     /* If execvp returns, there was an error. */
     restoreStatus = -1;
 freeLogFileOption:
-    if (logFileOption != NULL) {
+    if (NULL != logFileOption) {
         JLI_MemFree((void *)logFileOption);
         logFileOption = NULL;
     }
-freeLogLevelOption:
-    if (logLevelOption != NULL) {
-        JLI_MemFree((void *)logLevelOption);
-        logLevelOption = NULL;
-    }
 done:
     return restoreStatus;
 }

closed/src/java.base/share/native/libjli/criuhelpers.h

@@ -33,25 +33,6 @@
 #if defined(J9VM_OPT_CRAC_SUPPORT)
 #include <ctype.h>
 #include <sys/wait.h>
-
-static const char *
-getCommandLineOptionValue(const char *optionName, int argc, char **argv, int *error);
-
-static const char *
-getCheckpointDirectory(int argc, char **argv, int *error);
-
-static int
-getLogLevel(int argc, char **argv, int *error);
-
-static jboolean
-isUnprivilegedModeOn(int argc, char **argv, int *error);
-
-static const char *
-getLogFile(int argc, char **argv, int *error);
-
-static int
-restoreFromCheckpoint(const char *checkpointDirectory, int logLevel, jboolean unprivilegedModeOn, const char *logFile);
-
 void
 handleCRaCRestore(int argc, char **argv);
 #endif /* defined(J9VM_OPT_CRAC_SUPPORT) */