fix: close bug-1401 ctrlstatus leak, bug-1400 jobsub leak; harden bug-1396 harness

bug-1401: xce_free_ctrlstatus(s) was defined as free_CtrlStatus(s), which
only frees the fields inside xce_ctrlstatus_t. The real client reply path
(decode_CtrlStatus_ptr) calloc's the struct itself, so every ctrlstatus call
leaked sizeof(xce_ctrlstatus_t) (120 bytes). Existing unit tests never caught
it because they built/freed the struct via the raw stack-decode + bare
free_CtrlStatus() helpers, never exercising the actual client macro. Fixed
by switching the macro to free_CtrlStatus_ptr(&(s)) and added a regression
test that exercises the real heap-decode path; RED confirmed the exact
120-byte/1-alloc LeakSanitizer signature before the fix.

bug-1400: the originally reported SEGV in cmd_jobsub on malformed positional
input is stale — NULLSTR guards added 2025-11-05 already reject it cleanly.
The real residual defect was submit.start_info leaking on all 4 early-return
validation paths; fixed by routing them through a shared `goto cleanup`
(matches the existing goto-clean idiom in queue_ctrl).

bug-1396 (job-count anomaly after cold-restart-all, not yet root-caused):
instrumented test-commit-durability.sh's assert_job_count() to dump the live
joblist and ACKED_FILE ledger on any count mismatch instead of only the
count, per the bug's own recorded next step.

Verified: controller ASAN build + full controller_unit_test green; lib
xcelib_test 285/285 green, 0 ASAN leaks.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDWVNQLKxqTVHh42cH7rV3

src/controller/testsuite/test.c

@@ -1816,7 +1816,8 @@ int cmd_jobsub(int argc, char* argv[])
         default:
             printf(
                 "jobsub -u <user> -q <queue> -R <resreq> -c <command line>\n");
-            return -EINVAL;
+            rc = -EINVAL;
+            goto cleanup;
         }
     }
 
@@ -1827,17 +1828,20 @@ int cmd_jobsub(int argc, char* argv[])
 
     if (NULLSTR(submit.user)) {
         printf("you need to specify job user with -u\n");
-        return -EINVAL;
+        rc = -EINVAL;
+        goto cleanup;
     }
 
     if (NULLSTR(submit.queue)) {
         printf("you need to specify job queue with -q\n");
-        return -EINVAL;
+        rc = -EINVAL;
+        goto cleanup;
     }
 
     if (NULLSTR(submit.start_info->cmdline)) {
         printf("you need to specify job command with -c\n");
-        return -EINVAL;
+        rc = -EINVAL;
+        goto cleanup;
     }
 
     if (NULLSTR(submit.resreq)) {
@@ -1866,6 +1870,7 @@ int cmd_jobsub(int argc, char* argv[])
     jobsubmit.array_size = arsize;
     rc = xcecli_jobsubmit(active_ctx->ctx, &jobsubmit, &jobid);
 
+cleanup:
     FREEUP(submit.user);
     FREEUP(submit.queue);
     FREEUP(submit.resreq);

src/include/client.h

@@ -114,9 +114,12 @@ extern int             xcecli_jobinfo(xcecli_ctx_t, uint64_t, xce_job_t**);
 #define xcecli_free_queues(nqueues, queues) xce_free_queues(nqueues, queues)
 #define xcecli_free_groups(ngroups, groups) xce_free_groups(ngroups, groups)
 #define xcecli_free_clusterconf(conf)       xce_free_clusterconf(conf)
-/* ctrlstatus reply is freed by the generated codec free (free_CtrlStatus,
+/* ctrlstatus reply is freed by the generated codec free. free_CtrlStatus()
- * declared in the generated controller.capnp.h). */
+ * only frees the fields INSIDE the struct; the xce_ctrlstatus_t itself is
-#define xce_free_ctrlstatus(s)              free_CtrlStatus(s)
+ * calloc'd by decode_CtrlStatus_ptr() and must be freed via free_CtrlStatus_ptr()
+ * (which also nulls the caller's pointer) or every call leaks sizeof(xce_ctrlstatus_t)
+ * (bug-1401). */
+#define xce_free_ctrlstatus(s)              free_CtrlStatus_ptr(&(s))
 #define xcecli_free_ctrlstatus(s)           xce_free_ctrlstatus(s)
 
 #endif

src/lib/testsuite/test_ctrlstatus.c

@@ -192,6 +192,63 @@ static void test_ctrlstatus_roundtrip_unknown_primary(void)
     printf("test_ctrlstatus_roundtrip_unknown_primary: PASS\n");
 }
 
+/* bug-1401: xce_free_ctrlstatus() must free the heap-allocated xce_ctrlstatus_t
+ * itself, not just the fields inside it. decode_CtrlStatus_ptr() (the real
+ * client reply path, via xce_decode -> ... -> xce_ctrlstatus) calloc's the
+ * struct; free_CtrlStatus() alone only frees the inner strings/peers list and
+ * leaks the struct. This mirrors that allocation path (decode_CtrlStatus_ptr,
+ * not the stack-struct decode_CtrlStatus used by the roundtrip tests above)
+ * and relies on ASan leak detection to catch a regression. */
+static void test_xce_free_ctrlstatus_frees_the_struct_itself(void)
+{
+    xce_ctrlpeer_t   p0 = { .name = "ctrl-a", .host = "10.0.0.1", .port = 9991 };
+    xce_ctrlpeer_t*  peerlist[1] = { &p0 };
+    xce_ctrlstatus_t in;
+    memset(&in, 0, sizeof(in));
+    in.clusterName   = "lab-cluster";
+    in.primaryName   = "ctrl-a";
+    in.responderName = "ctrl-a";
+    in.configChecksum = "deadbeef";
+    in.peers.count   = 1;
+    in.peers.list    = peerlist;
+    in.peerCount     = 1;
+
+    struct capn          c;
+    struct capn_segment* cs;
+    struct CtrlStatus    cap_in;
+    CtrlStatus_ptr       sp;
+
+    capn_init_malloc(&c);
+    cs = capn_root(&c).seg;
+    memset(&cap_in, 0, sizeof(cap_in));
+    encode_CtrlStatus(cs, &cap_in, &in);
+    sp = new_CtrlStatus(cs);
+    write_CtrlStatus(&cap_in, sp);
+    capn_setp(capn_root(&c), 0, sp.p);
+
+    size_t   sz  = capn_size(&c);
+    uint8_t* buf = (uint8_t*)malloc(sz);
+    TEST_ASSERT_NOT_NULL(buf);
+    capn_write_mem(&c, buf, sz, 0);
+    capn_free(&c);
+
+    struct capn dc;
+    CtrlStatus_ptr dp;
+    capn_init_mem(&dc, buf, sz, 0);
+    dp.p = capn_getp(capn_root(&dc), 0, 1);
+
+    xce_ctrlstatus_t* out = NULL;
+    decode_CtrlStatus_ptr(&out, dp);
+    TEST_ASSERT_NOT_NULL(out);
+
+    xce_free_ctrlstatus(out);
+
+    capn_free(&dc);
+    free(buf);
+
+    printf("test_xce_free_ctrlstatus_frees_the_struct_itself: PASS\n");
+}
+
 /* ── no reachable controller ⇒ typed failure (Req 7.3) ──────────────────── */
 
 static void test_xcecli_ctrlstatus_null_ctx_typed_failure(void)
@@ -396,6 +453,7 @@ void test_ctrlstatus(void)
     RUN_TEST(test_handle_reply_ctrlstatus_does_not_free_foreign_slot);
     RUN_TEST(test_ctrlstatus_roundtrip_all_fields);
     RUN_TEST(test_ctrlstatus_roundtrip_unknown_primary);
+    RUN_TEST(test_xce_free_ctrlstatus_frees_the_struct_itself);
     RUN_TEST(test_xcecli_ctrlstatus_null_ctx_typed_failure);
     RUN_TEST(test_xcecli_ctrlstatus_primary_null_ctx_typed_failure);
     RUN_TEST(test_xcecli_ctrlstatus_primary_null_out_typed_failure);

src/tools/test-commit-durability.sh

@@ -392,7 +392,15 @@ assert_job_count() {
         pass "$label: cluster lists exactly $expected jobs"
         return 0
     else
-        fail "$label: job count != $expected after recovery (see expect-jobs output above)"
+        # bug-1396: a bare count mismatch isn't enough to diagnose an extra-job
+        # anomaly — dump the live joblist and the harness's own ACKED_FILE
+        # ledger side by side so the diverging job ID is visible in the log
+        # instead of only the count.
+        yellow "  $label: job count mismatch — dumping live joblist vs ACKED_FILE for diagnosis"
+        xce_test_script "$(printf 'set masters %s\nclient\njoblist\n' "$MASTERS")"
+        echo "  --- ACKED_FILE (tracked committed jobs, deduplicated) ---"
+        sort -u "$ACKED_FILE" | sed 's/^/    /'
+        fail "$label: job count != $expected after recovery (see joblist/ACKED_FILE dump above)"
         return 1
     fi
 }