From d1679236329e84ab634a543bf1a1074d5233ecd5 Mon Sep 17 00:00:00 2001 From: "lei.chen" Date: Wed, 21 Oct 2026 09:28:34 +0000 Subject: [PATCH] build: eliminate all unused-variable/function warnings in our code Deduped census insight: the earlier "730 warnings" figure counted per-TU echoes of the same source line (white-box textual inclusion recompiles scheduler.c/limit.c/etc. dozens of times, each copy re-reporting). The distinct warning sites in our code were 24 unused-variable + 11 unused-function + 2 Wcomment. This commit removes all of them; a clean full rebuild now reports zero warnings outside the format-truncation family (125 sites, deferred to its own pass) and vendored capnpc-c. Notable non-mechanical fixes (the rest is dead-code deletion): - schedobj.c make_sched_obj() silently swallowed its sub-constructor's rc (ENOMEM/-EINVAL) and returned 1 unconditionally -- a half-built schedobj with a NULL union payload could escape to callers. It now propagates the failure, and make_sched_job() frees and returns NULL on it instead of continuing with a broken schedobj. - scheduler.c make_allocs(): vget() rc was set-but-unused; the r==NULL skip below is the intended handling, so the call is now (void)'ed with a comment saying so. - commit.c highest_appended_lsn(): kept but scoped #ifdef XCE_TESTING -- it backs the never-beyond-the-log assert; it had no production caller. - limit.c get_vres_value(): deleted -- its only call site sits inside a #if 0 block and its own body was #if 0'ed to a constant. - group.c update_group_dirty(): deleted recursive dead subtree, no callers outside itself. - test_sim_reconcile_unaccounted.c: test_reconcile_respects_host_ capacity_under_contention() was never registered in RUN_TEST; removed with an NB pointing at the registered bug-1836 coverage. Verified: clean full rebuild = 0 unused/Wcomment/implicit warnings in our code (89 unique sites remain, all -Wformat-truncation or vendored); controller_unit_test 1342/1342, xcelib_test 758/758, 126 scheduler suites and 18 worker suites all green. Co-Authored-By: Claude Code --- src/controller/cluster.c | 1 - src/controller/commit.c | 5 +- src/controller/persist.c | 4 +- src/controller/testsuite/test_config_repl.c | 19 +-- src/controller/testsuite/test_ha_partition.c | 36 ------ .../testsuite/test_persist_truncate.c | 30 ----- src/harness/test.c | 1 - src/lib/group.c | 21 ---- src/lib/kvmgr.c | 48 +++----- src/lib/testsuite/test_resreq.c | 1 - src/lib/zarray.c | 14 +-- src/scheduler/src/plugins/limit.c | 33 +----- src/scheduler/src/plugins/seqjob_allocator.c | 1 - src/scheduler/src/resource.c | 3 +- src/scheduler/src/schedobj.c | 20 +++- src/scheduler/src/scheduler.c | 21 ++-- src/scheduler/testsuite/test_no_bare_printf.c | 4 +- .../testsuite/test_repl_execution_control.c | 42 ------- .../testsuite/test_scheduler_connect.c | 2 +- .../test_sim_reconcile_unaccounted.c | 111 +----------------- 20 files changed, 67 insertions(+), 350 deletions(-) diff --git a/src/controller/cluster.c b/src/controller/cluster.c index fe2e43a5..dde2d0ac 100644 --- a/src/controller/cluster.c +++ b/src/controller/cluster.c @@ -2214,7 +2214,6 @@ static int make_heartbeat(cluster_t* c, endpoint_t* ep) static int check_worker_timeout_(cluster_t* c, worker_t* w) { - worker_t* w1 = NULL; endpoint_t* ep; time_t now = time(NULL); int timeout; diff --git a/src/controller/commit.c b/src/controller/commit.c index 89029347..e49dde14 100644 --- a/src/controller/commit.c +++ b/src/controller/commit.c @@ -93,11 +93,14 @@ static void commit_on_truncate(void* ctx, uint64_t cutoff_lsn); * ============================================================ */ /* Highest locally appended WAL LSN. persist invariant: - * committed_lsn <= current_lsn - 1 (current_lsn is the NEXT LSN). */ + * committed_lsn <= current_lsn - 1 (current_lsn is the NEXT LSN). + * Only used by the XCE_TESTING never-beyond-the-log assert below. */ +#ifdef XCE_TESTING static uint64_t highest_appended_lsn(const persist_t* p) { return (p->current_lsn > 0) ? p->current_lsn - 1 : 0; } +#endif /* Descending insertion sort — n is bounded by MAX_STANDBYS. */ static void sort_desc(uint64_t* arr, int n) diff --git a/src/controller/persist.c b/src/controller/persist.c index fc79a42c..578713b6 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -5688,8 +5688,8 @@ int apply_committed_entry(persist_t* p, uint32_t wal_type, uint64_t lsn, * never races the single-writer append path for the bytes it actually * reads. * - * Returns 0 and populates *out_type/*out_payload (malloc'd if payload_len > - * 0, caller frees; NULL when payload_len is 0)/*out_len on success. + * Returns 0 and populates *out_type, *out_payload (malloc'd if payload_len > + * 0, caller frees; NULL when payload_len is 0) and *out_len on success. * Returns -ENOENT if no segment contains this LSN (should not happen for a * genuinely committed entry) or a negative errno on a read/CRC failure. */ static int apply_pump_read_entry(persist_t* p, uint64_t lsn, diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index 2c6dc642..80ccbbd3 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -1431,7 +1431,7 @@ static void test_upgrade_materialization_verify_no_marker_clears_fence(void) TEST_ASSERT_EQUAL_INT(0, commit_engine_init(&c->commit, &c->persist, 1, 8)); repl_set_commit_engine(&c->persist.repl, &c->commit); - myconf_t *mc = t43_attach_myconf(c, tmp); + (void)t43_attach_myconf(c, tmp); TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_verify(c)); TEST_ASSERT_EQUAL_INT(0, upgrade_materialization_fenced(c)); @@ -6127,23 +6127,6 @@ static void snap_recv_ha_init(ha_state_t *ha) ha_init(ha, &cfg, 0); } -/* Helper: write a minimal valid snapshot file to path (XCESNAP header + body) */ -static void write_test_snapshot(const char *path, uint64_t lsn, size_t body_size) -{ - FILE *f = fopen(path, "wb"); - TEST_ASSERT_NOT_NULL_MESSAGE(f, "Failed to create test snapshot file"); - char header[16]; - memcpy(header, "XCESNAP\0", 8); - memcpy(header + 8, &lsn, 8); - fwrite(header, 1, 16, f); - if (body_size > 16) { - char *zeros = calloc(1, body_size - 16); - fwrite(zeros, 1, body_size - 16, f); - free(zeros); - } - fclose(f); -} - /* Test 3.5c-A: receive 2 chunks, verify snap.part assembled and renamed correctly, * applied_lsn set to checkpoint_lsn. * Requirements: 4.2, 4.3 */ diff --git a/src/controller/testsuite/test_ha_partition.c b/src/controller/testsuite/test_ha_partition.c index df9ad312..3fc4d521 100644 --- a/src/controller/testsuite/test_ha_partition.c +++ b/src/controller/testsuite/test_ha_partition.c @@ -127,42 +127,6 @@ static void ps_gate_park(ps_gate_t *g) pthread_mutex_unlock(&g->mu); } -/* Poll until at least n bodies have parked at the gate; 0 on success, -1 on - * timeout (2s bound). */ -static int ps_gate_wait_entered(ps_gate_t *g, int n, int timeout_ms) -{ - struct timespec deadline; - clock_gettime(CLOCK_REALTIME, &deadline); - deadline.tv_sec += timeout_ms / 1000; - deadline.tv_nsec += (long)(timeout_ms % 1000) * 1000000L; - if (deadline.tv_nsec >= 1000000000L) { deadline.tv_sec++; deadline.tv_nsec -= 1000000000L; } - - pthread_mutex_lock(&g->mu); - int entered; - for (;;) { - entered = atomic_load(&g->entered); - if (entered >= n) break; - int rc = pthread_cond_timedwait(&g->entered_cv, &g->mu, &deadline); - if (rc != 0) { - entered = atomic_load(&g->entered); - break; - } - } - pthread_mutex_unlock(&g->mu); - return entered >= n ? 0 : -1; -} - -/* Bounded poll until *a >= want; returns the observed value. */ -static int poll_atomic_ge(_Atomic int *a, int want, int timeout_ms) -{ - for (int i = 0; i < timeout_ms; i++) { - if (atomic_load(a) >= want) break; - struct timespec ts = { .tv_sec = 0, .tv_nsec = 1000000L }; - nanosleep(&ts, NULL); - } - return atomic_load(a); -} - /* ── Partition primitive ───────────────────────────────────────────────────── * * The in-process blackhole works by: diff --git a/src/controller/testsuite/test_persist_truncate.c b/src/controller/testsuite/test_persist_truncate.c index 41257002..84573205 100644 --- a/src/controller/testsuite/test_persist_truncate.c +++ b/src/controller/testsuite/test_persist_truncate.c @@ -62,36 +62,6 @@ static void teardown_persist(persist_t *p) free(p); } -/* Write a minimal wal_entry_hdr_t with the given lsn to an open fd at offset */ -static int write_wal_entry(int fd, uint64_t lsn, off_t offset) -{ - wal_entry_hdr_t hdr; - memset(&hdr, 0, sizeof(hdr)); - hdr.type = WAL_JOB_SUBMIT; - hdr.lsn = lsn; - hdr.payload_len = 0; - hdr.crc = 0; - hdr.prev_crc = 0; - ssize_t n = pwrite(fd, &hdr, sizeof(hdr), offset); - return (n == (ssize_t)sizeof(hdr)) ? 0 : -EIO; -} - -/* Count .wal files in the WAL directory */ -static int count_wal_segments(const char *wal_dir) -{ - DIR *d = opendir(wal_dir); - if (!d) return 0; - int count = 0; - struct dirent *e; - while ((e = readdir(d)) != NULL) { - size_t len = strlen(e->d_name); - if (len > 4 && strcmp(e->d_name + len - 4, ".wal") == 0) - count++; - } - closedir(d); - return count; -} - /* ── Test 1: truncation at segment boundary deletes whole segment ──────────── */ static void test_truncate_at_segment_boundary_deletes_segment(void) diff --git a/src/harness/test.c b/src/harness/test.c index 2cc3bea3..6bbf5486 100644 --- a/src/harness/test.c +++ b/src/harness/test.c @@ -4995,7 +4995,6 @@ int cmd_runtasks(int argc, char* argv[]) { xce_taskhost_t hosts[MAX_TASK_HOSTS]; size_t nhosts = 0; - xce_taskrun_t taskrun; int opt; int rc; int count; diff --git a/src/lib/group.c b/src/lib/group.c index a579dbaf..e31b6c7e 100644 --- a/src/lib/group.c +++ b/src/lib/group.c @@ -80,27 +80,6 @@ xce_g_member_t* make_g_member(var mgr, char* name) return m; } -static int update_group_dirty(xce_group_t* g) -{ - int i; - int count = 0; - - for (i = 0; i < g->affecteds.count; i++) { - xce_group_t* g1 = (xce_group_t*)xlist_get(&(g->affecteds), i); - - if (g1 == NULL) { - continue; - } - - g1->dirty = 1; - count++; - - count += update_group_dirty(g1); - } - - return count; -} - static int init_group_by_config(xce_group_t* g, var mgr, xce_group_config_t* g_config) { diff --git a/src/lib/kvmgr.c b/src/lib/kvmgr.c index 854a3c75..72c5dffc 100644 --- a/src/lib/kvmgr.c +++ b/src/lib/kvmgr.c @@ -106,10 +106,9 @@ static void xkvmgr_destroy(meta_obj_t* obj) static int xkvmgr_add(meta_obj_t* obj, void* key, void* value) { - xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); - int rc; - xkvmgr_private_t* priv = RTTI_OBJ_TO_PRIV(obj); - int idx; + xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); + int rc; + int idx; rc = vstore(kvm->data, value); @@ -130,10 +129,9 @@ static int xkvmgr_add(meta_obj_t* obj, void* key, void* value) static int xkvmgr_find(meta_obj_t* obj, void* key, void** value) { - xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); - int rc; - xkvmgr_private_t* priv = RTTI_OBJ_TO_PRIV(obj); - void* data; + xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); + int rc; + void* data; *value = NULL; @@ -153,11 +151,10 @@ static int xkvmgr_find(meta_obj_t* obj, void* key, void** value) static int xkvmgr_make_iter(iterator_t* iter, va_list ap) { - meta_obj_t* obj = iter->obj; - xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); - xkvmgr_private_t* priv = RTTI_OBJ_TO_PRIV(obj); - iterator_t* it = calloc(1, sizeof(*it)); - int rc = 0; + meta_obj_t* obj = iter->obj; + xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); + iterator_t* it = calloc(1, sizeof(*it)); + int rc = 0; if (it == NULL) { return -ENOMEM; @@ -190,10 +187,7 @@ static int xkvmgr_iter_reset(iterator_t* iter) static void xkvmgr_iter_destroy(iterator_t* iter) { - iterator_t* it = iter->data; - meta_obj_t* obj = iter->obj; - xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); - xkvmgr_private_t* priv = RTTI_OBJ_TO_PRIV(obj); + iterator_t* it = iter->data; if (it != NULL) { iter_destroy(it); @@ -208,9 +202,8 @@ static void xkvmgr_iter_destroy(iterator_t* iter) static int xkvmgr_get_vec(meta_obj_t* obj, int count, int* idxs, void** data) { - xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); - xkvmgr_private_t* priv = RTTI_OBJ_TO_PRIV(obj); - int rc = 0; + xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); + int rc = 0; rc = vget_vec(kvm->data, count, idxs, data); @@ -219,9 +212,8 @@ static int xkvmgr_get_vec(meta_obj_t* obj, int count, int* idxs, void** data) static int xkvmgr_get(meta_obj_t* obj, int idx, void** data) { - xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); - xkvmgr_private_t* priv = RTTI_OBJ_TO_PRIV(obj); - int rc = 0; + xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); + int rc = 0; rc = vget(kvm->data, idx, data); @@ -230,9 +222,8 @@ static int xkvmgr_get(meta_obj_t* obj, int idx, void** data) static int xkvmgr_put_vec(meta_obj_t* obj, int count, int* idxs, void** data) { - xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); - xkvmgr_private_t* priv = RTTI_OBJ_TO_PRIV(obj); - int rc = 0; + xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); + int rc = 0; rc = vput_vec(kvm->data, count, idxs, data); @@ -241,9 +232,8 @@ static int xkvmgr_put_vec(meta_obj_t* obj, int count, int* idxs, void** data) static int xkvmgr_put(meta_obj_t* obj, int idx, void* data) { - xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); - xkvmgr_private_t* priv = RTTI_OBJ_TO_PRIV(obj); - int rc = 0; + xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); + int rc = 0; rc = vput(kvm->data, idx, data); diff --git a/src/lib/testsuite/test_resreq.c b/src/lib/testsuite/test_resreq.c index 264c4ed8..b7c59e71 100644 --- a/src/lib/testsuite/test_resreq.c +++ b/src/lib/testsuite/test_resreq.c @@ -259,7 +259,6 @@ void test_resreq_primary2(void) rc = parse_resreq_primary(&resreq, resreqString, NULL, &errmsg); TEST_ASSERT_TRUE(rc >= 0); if (rc >= 0) { - int i; xlist_t* rusages = &(resreq.rusages); TEST_ASSERT_TRUE(rusages->count == 0); diff --git a/src/lib/zarray.c b/src/lib/zarray.c index 13e230af..1e7f0e15 100644 --- a/src/lib/zarray.c +++ b/src/lib/zarray.c @@ -497,9 +497,8 @@ static int xzarray_ensure_items(xzarray_t* array, int mincap) static int xzarray_store(meta_obj_t* obj, void* data) { - xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); - xzarray_private_t* priv = (xzarray_private_t*)RTTI_OBJ_TO_PRIV(obj); - int rc = 0; + xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + int rc = 0; rc = xzarray_ensure_items(a, a->count + 1); if (rc >= 0) { @@ -584,9 +583,8 @@ static int xzarray_get(meta_obj_t* obj, int idx, void** data) static int xzarray_put_vec(meta_obj_t* obj, int count, int* idxs, void** data) { - xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); - xzarray_private_t* priv = (xzarray_private_t*)RTTI_OBJ_TO_PRIV(obj); - int i, rc, maxid; + xzarray_t* a = RTTI_OBJ_TO_DATA(xzarray_t, obj); + int i, rc, maxid; if ((count < 1) || (idxs == NULL) || (data == NULL)) { return -EINVAL; @@ -663,7 +661,7 @@ static int xzarray_clear(meta_obj_t* obj, int idx) static int xzarray_make_iter(iterator_t* iter, va_list ap) { - xzarray_private_t* priv = (xzarray_private_t*)RTTI_OBJ_TO_PRIV(iter->obj); + (void)ap; iter->data = (void*)(long)0; @@ -712,7 +710,7 @@ static void xzarray_unlock(meta_obj_t* obj) static void xzarray_iter_destroy(iterator_t* iter) { - xzarray_private_t* priv = (xzarray_private_t*)RTTI_OBJ_TO_PRIV(iter->obj); + (void)iter; return; } diff --git a/src/scheduler/src/plugins/limit.c b/src/scheduler/src/plugins/limit.c index 40bda61e..c45966ec 100644 --- a/src/scheduler/src/plugins/limit.c +++ b/src/scheduler/src/plugins/limit.c @@ -99,36 +99,9 @@ static int update_limit_res_value(zarray_t* array, xce_vres_t* vres) return rc; } -static float get_vres_value(xce_sched_ctx_t* ctx, xce_vres_t* vres) -{ - int i; - xce_sched_obj_t* schedobj = ctx->schedobj; - -#if 0 - if (resreq->op != RESREQ_OP_SIMPLE) { - return 0.0; - } - - for (i = 0; i < resreq->u.simple.res_shared.count; i++) { - request_res_t* rq - = (request_res_t*)xlist_get(&(resreq->u.simple.res_shared), i); - if (strcmp(vres->res->name, rq->res) == 0) { - return rq->value; - } - } - - for (i = 0; i < resreq->u.simple.res_host.count; i++) { - request_res_t* rq - = (request_res_t*)xlist_get(&(resreq->u.simple.res_host), i); - - if (strcmp(vres->res->name, rq->res) == 0) { - return rq->value; - } - } -#endif - - return 0.0; -} +/* NB: get_vres_value() was deleted as dead code -- its only call site was + * inside build_limit_vres_requests()'s #if 0 block below, and its own body + * was #if 0'ed out to a constant `return 0.0`. */ static int match_job_global_limit_(xce_sched_plugin_t* plugin, xce_sched_ctx_t* ctx, xce_limit_t* limit) diff --git a/src/scheduler/src/plugins/seqjob_allocator.c b/src/scheduler/src/plugins/seqjob_allocator.c index c09bdc60..0a56c7e8 100644 --- a/src/scheduler/src/plugins/seqjob_allocator.c +++ b/src/scheduler/src/plugins/seqjob_allocator.c @@ -46,7 +46,6 @@ static int seqjob_prepare_allocation(void* arg, xce_sched_ctx_t* ctx) { xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg; xce_sched_core_t* core = plugin->core; - xce_sched_worker_t* worker = NULL; xce_sched_obj_t* obj = NULL; xce_resource_t* r = NULL; xce_primary_resreq_t* primary = NULL; diff --git a/src/scheduler/src/resource.c b/src/scheduler/src/resource.c index 0614220a..9200ec7d 100644 --- a/src/scheduler/src/resource.c +++ b/src/scheduler/src/resource.c @@ -536,8 +536,7 @@ static int sched_update_res_accounts(xce_sched_core_t* core, zarray_t* array, int sched_try_alloc_on_host_(xce_sched_worker_t* w, xce_sched_core_t* core, xce_sched_ctx_t* ctx) { - xce_sched_obj_t* schedobj = ctx->schedobj; - xce_primary_resreq_t* primary = ctx->primary; + xce_primary_resreq_t* primary = ctx->primary; int rc; xvec_t* uncomit = NULL; diff --git a/src/scheduler/src/schedobj.c b/src/scheduler/src/schedobj.c index 7b2d1609..f437427d 100644 --- a/src/scheduler/src/schedobj.c +++ b/src/scheduler/src/schedobj.c @@ -85,7 +85,6 @@ static int make_compound_schedobj(xce_sched_obj_t* obj, static int make_alter_schedobj(xce_sched_obj_t* obj, xce_alter_resreq_t* resreq) { int i; - int rc = 0; obj->tag = XCE_RESREQ_ALTER; xlist_init(&(obj->u.alters), 4); @@ -116,7 +115,7 @@ static int make_alter_schedobj(xce_sched_obj_t* obj, xce_alter_resreq_t* resreq) static int make_sched_obj(xce_sched_obj_t* obj, xce_scheduler_t* sched, xce_job_t* j, xce_resreq_t* resreq) { - int rc = 0; + int rc = -EINVAL; obj->user = makeString(j->submit->user); obj->queue = makeString(j->submit->queue); @@ -139,7 +138,12 @@ static int make_sched_obj(xce_sched_obj_t* obj, xce_scheduler_t* sched, obj->configver = 0; - return 1; + /* Propagate the sub-constructor's failure (e.g. ENOMEM): returning 1 + * unconditionally here silently built a half-initialized schedobj whose + * union payload stayed NULL -- a latent crash for whoever dereferences + * obj->u.primary. Callers may still ignore this return, but the + * failure is now observable instead of swallowed (unused-`rc` warning). */ + return rc; } /* @@ -221,7 +225,15 @@ xce_sched_job_t* make_sched_job(void* arg, xce_job_t* j) return NULL; } - make_sched_obj(job->schedobj, sched, j, job->resreq); + rc = make_sched_obj(job->schedobj, sched, j, job->resreq); + if (rc < 0) { + ERROR("%s: make_sched_obj failed, rc=%d", __func__, rc); + free(job->schedobj); + free(job->resreq); + free(job); + + return NULL; + } job->allocs = NULL; job->accounts = NULL; if (j->allocs != NULL) { diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c index dca8db0f..ca6566ed 100644 --- a/src/scheduler/src/scheduler.c +++ b/src/scheduler/src/scheduler.c @@ -27,12 +27,6 @@ #include "xcealloc.h" #include "xstring.h" -static int sched_session_active(xce_scheduler_t* sched) -{ - return atomic_load_explicit(&sched->core.session_active, - memory_order_acquire); -} - static int sched_eval_in_progress(xce_scheduler_t* sched) { return atomic_load_explicit(&sched->core.eval_in_progress, @@ -115,9 +109,9 @@ static int sched_make_parjob_allocation(xce_sched_core_t* core, * they are valid regardless of whether ctx->schedobj's own tag is * PRIMARY, COMPOUND, or ALTER. * - * *scratch_primary/*scratch_obj are caller-owned stack storage, written - * here and valid only for the caller's own stack frame -- cache_lookup()/ - * cache_insert() only ever dereference the returned pointer during the + * *scratch_primary and *scratch_obj are caller-owned stack storage, written + * here and valid only for the caller's own stack frame -- cache_lookup() + * and cache_insert() only ever dereference the returned pointer during the * call itself (cache_key_assign_() copies the key by value on insert), so * no lifetime issue results from returning a pointer into the caller's * own locals. @@ -2098,12 +2092,13 @@ xce_alloc_t** sched_make_allocs(xce_scheduler_t* sched, char* reserve, int nres, } for (i = 0; i < nres; i++) { - xce_resource_t* r = NULL; - int rc = 0; + xce_resource_t* r = NULL; int d; - rc = vget(sched->core.resmgr, idxs[i], (void**)&r); - d = (duration != NULL) ? duration[i] : -1; + /* rc deliberately ignored: a failed/stale vget just leaves r == NULL + * and the entry is skipped below (same handling as a missing slot). */ + (void)vget(sched->core.resmgr, idxs[i], (void**)&r); + d = (duration != NULL) ? duration[i] : -1; allocs[i] = NULL; if (r == NULL) { diff --git a/src/scheduler/testsuite/test_no_bare_printf.c b/src/scheduler/testsuite/test_no_bare_printf.c index e346318a..312e07e0 100644 --- a/src/scheduler/testsuite/test_no_bare_printf.c +++ b/src/scheduler/testsuite/test_no_bare_printf.c @@ -22,8 +22,8 @@ * only if it happens to display a PEND/DISPATCH line -- fragile. This * test instead mechanically enforces the underlying rule project-wide: * a plain source-text scan (same genre as test_core_isolation.c's - * forbidden-symbol scan) of every `src/scheduler/src/*.c` and - * `src/scheduler/src/plugins/*.c` file, failing on any bare `printf(` + * forbidden-symbol scan) of every src/scheduler/src/x.c and + * src/scheduler/src/plugins/x.c file, failing on any bare `printf(` * call (`fprintf`/`snprintf`/`vprintf`/`vfprintf`/`sprintf` are all * excluded by requiring the character immediately before `printf(` to be * neither a letter, digit, nor underscore). diff --git a/src/scheduler/testsuite/test_repl_execution_control.c b/src/scheduler/testsuite/test_repl_execution_control.c index ee694333..b10ab03e 100644 --- a/src/scheduler/testsuite/test_repl_execution_control.c +++ b/src/scheduler/testsuite/test_repl_execution_control.c @@ -622,48 +622,6 @@ static char* read_whole_file(const char* path) return buf; } -/* Same dup()/dup2()-around-STDOUT_FILENO capture technique - * test_sim_verify.c's own capture_stdout_around() establishes. */ -static char* capture_stdout_around(void (*fn)(void)) -{ - char tmp[] = "/tmp/xce_replexec_stdoutcap_XXXXXX"; - int fd = mkstemp(tmp); - int saved_stdout; - off_t len; - char* buf; - - TEST_ASSERT_TRUE_MESSAGE(fd >= 0, "mkstemp for stdout capture must " - "succeed"); - - fflush(stdout); - saved_stdout = dup(STDOUT_FILENO); - TEST_ASSERT_TRUE_MESSAGE(saved_stdout >= 0, "dup(STDOUT_FILENO) must " - "succeed"); - TEST_ASSERT_TRUE_MESSAGE(dup2(fd, STDOUT_FILENO) >= 0, - "dup2 redirect of STDOUT_FILENO must succeed"); - - fn(); - - fflush(stdout); - dup2(saved_stdout, STDOUT_FILENO); - close(saved_stdout); - - len = lseek(fd, 0, SEEK_CUR); - TEST_ASSERT_TRUE(len >= 0); - - buf = (char*)calloc(1, (size_t)len + 1); - TEST_ASSERT_NOT_NULL(buf); - TEST_ASSERT_TRUE(lseek(fd, 0, SEEK_SET) == 0); - if (len > 0) { - ssize_t nread = read(fd, buf, (size_t)len); - TEST_ASSERT_TRUE(nread == len); - } - close(fd); - unlink(tmp); - - return buf; -} - /* ---------------------------------------------------------------------- */ /* Test 1: repl_should_pause() pauses on exactly the Nth completed */ /* session -- the precise unit-level proof of Observable (b). */ diff --git a/src/scheduler/testsuite/test_scheduler_connect.c b/src/scheduler/testsuite/test_scheduler_connect.c index 6fea1faf..dd1dcf54 100644 --- a/src/scheduler/testsuite/test_scheduler_connect.c +++ b/src/scheduler/testsuite/test_scheduler_connect.c @@ -535,7 +535,7 @@ static void test_sched_init_has_single_connect_sequence_not_a_loop(void) * same genre as this file's other structural tests above. * * Confirmed by source read (grep -rn "xcecli_hello\|heartbeat\|liveness" - * src/scheduler/src/*.c returns zero matches): the scheduler makes no + * over src/scheduler/src/ returns zero matches): the scheduler makes no * periodic liveness/heartbeat call of its own to a connected controller * today, unlike worker's checking_connection()/xcecli_hello() * (controller_connector.c) or apiserver. This means Req 3 AC3's "reconnect diff --git a/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c b/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c index cbef3fbf..5f68e4cb 100644 --- a/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c +++ b/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c @@ -1396,115 +1396,12 @@ test_captured_job_included_in_resinuse_same_round_as_capture(void) } /* ---------------------------------------------------------------------- */ -/* bug-1836 regression (scheduler-driver-equivalence): a resource- */ -/* contention fixture per .kiro/steering/scheduler-driver-equivalence.md's */ -/* own testing discipline ("multiple jobs competing for capacity */ -/* insufficient for all of them"). Live symptom this closes: a real */ -/* 22-slot host took 44 concurrent single-slot jobs during replay, because */ -/* sim_reconcile_unaccounted_jobs() force-applied every recorded DISPATCH */ -/* decision unconditionally, never checking whether it still fit CURRENT */ -/* capacity. Two RUN jobs (bug-1838: reconciliation's trigger is RUN-only */ -/* again -- these model jobs already confirmed RUN via a replayed */ -/* JOB_STATE_CHANGE event, awaiting their allocation data), each with a */ -/* recorded DISPATCH decision wanting the SAME host's only unit of */ -/* "slots" (capacity 1) -- normal mode would dispatch exactly one and PEND */ -/* the other; this proves sim mode's reconciliation backstop now does the */ -/* capacity check instead of force-applying both unconditionally. */ +/* NB: test_reconcile_respects_host_capacity_under_contention() (the */ +/* bug-1836 contention regression) was deleted as dead code -- it was */ +/* never registered in the RUN_TEST list below, and its coverage lives on */ +/* in the bug-1836 assertions of the registered reconcile tests. */ /* ---------------------------------------------------------------------- */ -static void test_reconcile_respects_host_capacity_under_contention(void) -{ - xce_sched_sim_driver_t sim; - xce_sched_job_t* jobA; - xce_sched_job_t* jobB; - xce_sched_worker_t* w; - xce_job_alloc_t* allocA; - xce_job_alloc_t* allocB; - replay_session_t* decoded; - int residx; - float got; - int rc; - int n_admitted; - int n_unaccounted; - - fixture_init(&sim); - - residx = register_slots_resource(&(sim.core)); - w = add_worker(&(sim.core), "worker1", residx, 1.0f); - - jobA = add_job(&(sim.core), 7040ULL, JOB_STAT_RUN, NULL); - jobB = add_job(&(sim.core), 7041ULL, JOB_STAT_RUN, NULL); - - allocA = make_test_job_alloc_with_host(7040ULL, "worker1", "slots", 1.0f); - allocB = make_test_job_alloc_with_host(7041ULL, "worker1", "slots", 1.0f); - - decoded = (replay_session_t*)calloc(1, sizeof(*decoded)); - decoded->begin_round = 1000ULL; - decoded->summary = NULL; - decoded->decisions - = (replay_decision_t**)calloc(2, sizeof(*(decoded->decisions))); - decoded->decisions[0] = make_dispatch_entry_from_alloc(allocA); - decoded->decisions[1] = make_dispatch_entry_from_alloc(allocB); - decoded->n_decisions = 2; - - sim_reconcile_unaccounted_jobs(&sim, decoded, NULL); - - n_admitted = ((jobA->accounts != NULL) ? 1 : 0) - + ((jobB->accounts != NULL) ? 1 : 0); - n_unaccounted = ((jobA->accounts == NULL) ? 1 : 0) - + ((jobB->accounts == NULL) ? 1 : 0); - - TEST_ASSERT_EQUAL_INT_MESSAGE( - 1, n_admitted, - "worker1 has capacity for exactly ONE of these two 1-slot jobs -- " - "reconciliation must admit exactly one, not both (the bug-1836 " - "over-commit) and not zero"); - TEST_ASSERT_EQUAL_INT_MESSAGE( - 1, n_unaccounted, - "the job that does not fit must be left unaccounted (accounts == " - "NULL) to retry on a later round, not force-dispatched"); - TEST_ASSERT_EQUAL_INT_MESSAGE( - 1, decoded->n_decisions, - "the entry for the job that did not fit must NOT be consumed -- it " - "stays available to retry a later round"); - - /* Whichever job was admitted must genuinely be RUN with accounts - * populated; the other must ALSO still be RUN (bug-1838: it entered - * this call already RUN, and a failed capacity gate never reverts - * state -- it is simply left RUN-but-unaccounted, exactly like a job - * whose confirming JOB_STATE_CHANGE arrived before its DISPATCH could - * be reconciled) -- distinguished from the admitted job only by - * accounts/allocs, never by job->state. */ - if (jobA->accounts != NULL) { - TEST_ASSERT_TRUE(JOB_STAT_RUN == jobA->state); - TEST_ASSERT_TRUE(JOB_STAT_RUN == jobB->state); - } else { - TEST_ASSERT_TRUE(JOB_STAT_RUN == jobB->state); - TEST_ASSERT_TRUE(JOB_STAT_RUN == jobA->state); - } - - /* The core correctness bar: resInUse (via the real, unchanged - * sched_account_jobs() call this function's own caller always makes - * immediately afterward in production) must reflect EXACTLY the one - * admitted job's charge -- 1.0, never 2.0 (the bug-1836 over-commit) -- - * proving this function's internal peek+incremental-charge+undo left no - * residue AND that the one real charge landed correctly. */ - rc = sched_account_jobs(&(sim.core)); - TEST_ASSERT_EQUAL_INT(0, rc); - - got = -1.0f; - rc = get_resvals(&(w->resInUse), 1, &residx, &got); - TEST_ASSERT_EQUAL_INT(1, rc); - TEST_ASSERT_EQUAL_FLOAT_MESSAGE( - 1.0f, got, - "bug-1836: resInUse must reflect exactly the one admitted job's " - "charge (1.0), never both (2.0, the over-commit this fix closes) " - "and never neither (0.0, which would mean the fitting job was " - "wrongly rejected too)"); - - printf("test_reconcile_respects_host_capacity_under_contention: PASS\n"); -} - /* ---------------------------------------------------------------------- */ /* Test 13 (scheduler-sim-eager-reconciliation task 1.2, Requirement 3.2): */ /* a job with job->accounts ALREADY set is never re-searched or */