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 <noreply@anthropic.com>
This commit is contained in:
lei.chen
parent
f1801fbef9
commit
d167923632
20 changed files with 67 additions and 350 deletions
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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). */
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 */
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue