Merge pull request #201 from chu11/issue197_segfault

powerman: correct potential segfault
chaos · Sep 10, 2024 · ba0abce · ba0abce
2 parents 16ff45a + b45e68f
commit ba0abce
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 33 deletions.
diff --git a/src/powerman/device.c b/src/powerman/device.c
@@ -88,7 +88,7 @@ typedef struct {
  */
 #define MAX_LEVELS 2
 typedef struct {
-    int com;                    /* one of the PM_* above */
+    int com;                    /* one of the PM_* script types */
     List exec;                  /* stack of ExecCtxs (outer block is first) */
     ActionCB complete_fun;      /* callback for action completion */
     VerbosePrintf vpf_fun;      /* callback for device telemetry */
@@ -1178,7 +1178,7 @@ static bool _process_setresult(Device *dev, Action *act, ExecCtx *e)
                 arg->val = xstrdup(str);
             }
 
-            if (result != RT_SUCCESS) {
+            if (arg && result != RT_SUCCESS) {
                 char strbuf[1024];
                 snprintf(strbuf, sizeof(strbuf), "%s", arg->val);
                 /* remove trailing carriage return or newline */

diff --git a/src/redfishpower/redfishpower.c b/src/redfishpower/redfishpower.c
@@ -1520,6 +1520,39 @@ static void cleanup_powermsg(void **x)
     }
 }
 
+static void output_curl_error(struct CURLMsg *cmsg, struct powermsg *pm)
+{
+    if (cmsg->data.result == CURLE_HTTP_RETURNED_ERROR) {
+        /* N.B. curl returns this error code for all response
+         * codes >= 400.  So gotta dig in more.
+         */
+        long code;
+
+        if (curl_easy_getinfo(cmsg->easy_handle,
+                              CURLINFO_RESPONSE_CODE,
+                              &code) != CURLE_OK)
+            printf("%s: %s\n", pm->plugname, "http error");
+        if (code == 400)
+            printf("%s: %s\n", pm->plugname, "bad request");
+        else if (code == 401)
+            printf("%s: %s\n", pm->plugname, "unauthorized");
+        else if (code == 404)
+            printf("%s: %s\n", pm->plugname, "not found");
+        else
+            printf("%s: %s (%ld)\n",
+                   pm->plugname,
+                   "http error",
+                   code);
+    }
+    else
+        printf("%s: %s\n",
+               pm->plugname,
+               curl_easy_strerror(cmsg->data.result));
+    if (verbose)
+        printf("%s: %s\n", pm->plugname,
+               curl_easy_strerror(cmsg->data.result));
+}
+
 static void shell(CURLM *mh)
 {
     int exitflag = 0;
@@ -1681,36 +1714,8 @@ static void shell(CURLM *mh)
                         err_exit(false, "private data not set in easy handle");
 
                     if (cmsg->data.result != 0) {
-                        if (cmsg->data.result == CURLE_HTTP_RETURNED_ERROR) {
-                            /* N.B. curl returns this error code for all response
-                             * codes >= 400.  So gotta dig in more.
-                             */
-                            long code;
-
-                            if (curl_easy_getinfo(cmsg->easy_handle,
-                                                  CURLINFO_RESPONSE_CODE,
-                                                  &code) != CURLE_OK)
-                                printf("%s: %s\n", pm->plugname, "http error");
-                            if (code == 400)
-                                printf("%s: %s\n", pm->plugname, "bad request");
-                            else if (code == 401)
-                                printf("%s: %s\n", pm->plugname, "unauthorized");
-                            else if (code == 404)
-                                printf("%s: %s\n", pm->plugname, "not found");
-                            else
-                                printf("%s: %s (%ld)\n",
-                                       pm->plugname,
-                                       "http error",
-                                       code);
-                        }
-                        else
-                            printf("%s: %s\n",
-                                   pm->plugname,
-                                   curl_easy_strerror(cmsg->data.result));
-                        if (verbose)
-                            printf("%s: %s\n", pm->plugname,
-                                   curl_easy_strerror(cmsg->data.result));
-
+                        if (pm->output_result)
+                            output_curl_error (cmsg, pm);
                         process_waiters(mh,
                                         pm->plugname,
                                         STATUS_ERROR);
@@ -1749,7 +1754,8 @@ static void shell(CURLM *mh)
             pm = zlistx_first(cpy);
             while (pm) {
                 if (hostlist_find(test_fail_power_cmd_hosts, pm->hostname) >= 0) {
-                    printf("%s: %s\n", pm->plugname, "error");
+                    if (pm->output_result)
+                        printf("%s: %s\n", pm->plugname, "error");
                     process_waiters(mh,
                                     pm->plugname,
                                     STATUS_ERROR);

diff --git a/t/t0037-cray-ex.t b/t/t0037-cray-ex.t
@@ -295,6 +295,38 @@ test_expect_success 'stop powerman daemon (crayexU)' '
 	wait
 '
 
+#
+# redfishpower hpe cray supercomputing ex chassis test - parent failure
+# see issue #197
+#
+
+test_expect_success 'create powerman.conf for chassis w/ 16 redfish nodes (crayexfail)' '
+	cat >powerman_cray_ex_fail.conf <<-EOT
+	listen "$testaddr"
+	include "$devicesdir/redfishpower-cray-ex.dev"
+	device "d0" "cray-ex" "$redfishdir/redfishpower -h cmm0,t[0-15] --test-mode --test-fail-power-cmd-hosts=cmm0 |&"
+	node "cmm0,perif[0-7],blade[0-7],t[0-15]" "d0"
+	EOT
+'
+test_expect_success 'start powerman daemon and wait for it to start (crayexfail)' '
+	$powermand -Y -c powerman_cray_ex_fail.conf &
+	echo $! >powermand.pid &&
+	$powerman --retry-connect=100 --server-host=$testaddr -d
+'
+test_expect_success 'powerman -q shows all unknown' '
+	$powerman -h $testaddr -q >test_crayexfail_query1.out &&
+	makeoutput "" "" "blade[0-7],cmm0,perif[0-7],t[0-15]" >test_crayexfail_query1.exp &&
+	test_cmp test_crayexfail_query1.exp test_crayexfail_query1.out
+'
+test_expect_success 'powerman -1 t0 dependency error (parent error)' '
+	test_must_fail $powerman -h $testaddr -1 t0 >test_crayexfail_on1.out 2>test_crayexfail_on1.err &&
+	grep "cannot perform on, dependency error" test_crayexfail_on1.err
+'
+test_expect_success 'stop powerman daemon (crayexfail)' '
+	kill -15 $(cat powermand.pid) &&
+	wait
+'
+
 test_done
 
 # vi: set ft=sh