fix(scheduler): repl_cmd_delete() rejects non-numeric/invalid breakpoint ids
delete <id> used atoi(), which silently coerces a non-numeric or trailing-garbage token to 0 instead of reporting an error, e.g. "delete abc" printed "no breakpoint with id 0" rather than rejecting the input. Switched to strtol()+endptr validation, mirroring the same "fail loud" discipline repl_cmd_step() already established in this file family. Added test_delete_non_numeric_id_rejected covering non-numeric, zero, negative, and trailing-garbage ids. Logged as bug-1761 (buglog.json entry already committed alongside an unrelated task's changes earlier in this branch's history). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
This commit is contained in:
parent
b639fae162
commit
140f0900b5
2 changed files with 59 additions and 4 deletions
|
|
@ -548,12 +548,28 @@ bool repl_cmd_delete(xce_sched_sim_driver_t* sim, const char* line)
|
|||
}
|
||||
|
||||
{
|
||||
int id = atoi(tokens[1]);
|
||||
/* strtol()+endptr, not atoi() -- same "fail loud" discipline
|
||||
* repl.c's repl_cmd_step() doc comment establishes: atoi() would
|
||||
* silently coerce a non-numeric token (e.g. a typo'd id) to 0
|
||||
* rather than reporting the input as invalid, misreporting a bad
|
||||
* argument as "no breakpoint with id 0". Breakpoint ids are
|
||||
* assigned via pre-incremented next_bp_id (repl_cmd_break(), this
|
||||
* file), so 1 is the smallest id that can ever legitimately exist. */
|
||||
long id;
|
||||
char* endptr;
|
||||
|
||||
if (!repl_breakpoint_remove(sim->repl_state, id)) {
|
||||
fprintf(stdout, "[sim] delete: no breakpoint with id %d\n", id);
|
||||
id = strtol(tokens[1], &endptr, 10);
|
||||
if ((endptr == tokens[1]) || (*endptr != '\0') || (id <= 0)) {
|
||||
fprintf(stdout,
|
||||
"[sim] delete: '%s' is not a valid breakpoint id -- usage: "
|
||||
"delete <id>|all\n", tokens[1]);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!repl_breakpoint_remove(sim->repl_state, (int)id)) {
|
||||
fprintf(stdout, "[sim] delete: no breakpoint with id %ld\n", id);
|
||||
} else {
|
||||
fprintf(stdout, "[sim] breakpoint %d deleted\n", id);
|
||||
fprintf(stdout, "[sim] breakpoint %ld deleted\n", id);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -420,6 +420,44 @@ static void test_delete_all_removes_all_breakpoints(void)
|
|||
printf("test_delete_all_removes_all_breakpoints: PASS\n");
|
||||
}
|
||||
|
||||
static void test_delete_non_numeric_id_rejected(void)
|
||||
{
|
||||
xce_sched_sim_driver_t sim;
|
||||
|
||||
memset(&sim, 0, sizeof(sim));
|
||||
/* xce_sched_core_init() is the same precondition xce_repl_start()
|
||||
* always has satisfied for it in the real driver flow (sched_sim_driver_start()
|
||||
* runs first) -- it also transitively calls xstring_table_init()
|
||||
* (core.c), which makeString() (breakpoint token/filter string copies,
|
||||
* repl_break.c) requires to not silently return NULL. */
|
||||
TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sim.core)));
|
||||
sim.repl_state = repl_state_ensure(&sim);
|
||||
TEST_ASSERT_NOT_NULL(sim.repl_state);
|
||||
|
||||
TEST_ASSERT_FALSE(repl_dispatch(&sim, "break JOB_PUSH"));
|
||||
TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints);
|
||||
|
||||
/* "abc" must be rejected outright, not silently coerced to id 0
|
||||
* (atoi()'s old behavior) and reported as "no breakpoint with id 0". */
|
||||
TEST_ASSERT_FALSE(repl_dispatch(&sim, "delete abc"));
|
||||
TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints);
|
||||
|
||||
/* "0" and negative ids can never be legitimately assigned
|
||||
* (repl_cmd_break()'s pre-incremented next_bp_id starts at 1) -- also
|
||||
* rejected as invalid, not treated as a well-formed-but-missing id. */
|
||||
TEST_ASSERT_FALSE(repl_dispatch(&sim, "delete 0"));
|
||||
TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints);
|
||||
TEST_ASSERT_FALSE(repl_dispatch(&sim, "delete -1"));
|
||||
TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints);
|
||||
|
||||
/* trailing garbage after a numeric prefix ("1x") is also rejected, not
|
||||
* truncated down to id 1. */
|
||||
TEST_ASSERT_FALSE(repl_dispatch(&sim, "delete 1x"));
|
||||
TEST_ASSERT_EQUAL_INT(1, sim.repl_state->nbreakpoints);
|
||||
|
||||
printf("test_delete_non_numeric_id_rejected: PASS\n");
|
||||
}
|
||||
|
||||
static void test_info_break_lists_breakpoints_with_filters(void)
|
||||
{
|
||||
xce_sched_sim_driver_t sim;
|
||||
|
|
@ -1997,6 +2035,7 @@ int main(void)
|
|||
RUN_TEST(test_break_malformed_filter_rejected);
|
||||
RUN_TEST(test_delete_by_id_removes_one_breakpoint);
|
||||
RUN_TEST(test_delete_all_removes_all_breakpoints);
|
||||
RUN_TEST(test_delete_non_numeric_id_rejected);
|
||||
RUN_TEST(test_info_break_lists_breakpoints_with_filters);
|
||||
|
||||
RUN_TEST(test_event_breakpoint_matches_jobid_filter);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue