lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(job-array-one-based-index): parse_jobid on top of parse_jobsel (task 1.2)

Browse source 
parse_jobid() keeps its signature but is now bounded and NULL-safe: a
bare identity returns the exact ID, [k] the element ID, and a range, any
rejected selection (5.7, 5.8, 5.14, 5.15) or a NULL argument returns
-EINVAL with *jobid = 0. Fixes the strlen-before-NULL-check crash and
the atoi overflow; regex parsing is gone.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-01 09:15:48 +00:00
parent 6e6c599d27
commit d027410304
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
3 changed files with 119 additions and 57 deletions
Download patch file Download diff file Expand all files Collapse all files

5
.kiro/specs/job-array-one-based-index/tasks.md
View file

@ -9,7 +9,7 @@
- Done when the libxce suite runs the conformance table with every row passing, including `X[0]`, `X[01]`, the width cases (`0-0-T-1048576[1]`) and the precedence rows
- _Requirements: 1.5, 5.7, 5.8, 5.12, 5.14, 5.15, 8.1_
- [ ] 1.2 Re-implement the existing identity parse on top of the new parser
- [x] 1.2 Re-implement the existing identity parse on top of the new parser
- Keep its signature; make it NULL-safe, bounded, and reject element ranges and every rejected selection with an invalid-argument result
- Keep the existing format/parse round-trip tests passing unchanged
- Callers outside libxce (the harness single-job commands and the scheduler REPL, whose fall-back to plain number parsing is accepted unchanged) see only the stricter acceptance
@ -236,3 +236,6 @@
- Done when both skills report PASS, the manual flow shows the expected elements and `XCE_JOBINDEX` values, and the three bug records are fixed
- _Requirements: 1.1, 1.10, 5.1, 5.2, 6.1, 8.4_
- _Depends: 3.2, 7.4, 8.2, 8.3, 9.1, 9.2_
## Implementation Notes
- 1.2: `REGEX_JOBID`, `REGEX_JOBARRAY_ID` and `N_MAX_FIELDS_JOBID` (`src/include/config.h`) are now unused, and comments still describe regex-based `parse_jobid` in `src/scheduler/src/repl.c:143`, `src/harness/testsuite/test_test_cli.c:2324-2326` and `src/frontend/xcecli/common/format/jobid.go:86-93`; clean these up in 4.3 / 8.1 / 9.x where those files are touched. A direct `parse_jobid("...[0]")` accept test is a cheap addition when next touching `test_jobsel.c`.

97
src/lib/testsuite/test_jobsel.c
View file

@ -3,13 +3,15 @@
*
* Runs the identity rows of the shared `selection-cases` conformance table
* (design.md, Testing Strategy) against parse_jobsel(), plus the helpers
* is_array_tag(), jobsel_element_id() and jobsel_strerror().
* is_array_tag(), jobsel_element_id() and jobsel_strerror(), and (task 1.2)
* parse_jobid() re-implemented on top of parse_jobsel().
*
* The C surface has no decimal keys and no name selections, so the 5.17 and
* name rows of the shared table do not apply here. `X` below stands for a
* valid, array-capable written identity (`3-5-1700000000-65536`: sequence is
* a multiple of 32768).
*/
#include <errno.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
@ -259,6 +261,96 @@ static void test_jobsel_strerror(void)
TEST_ASSERT_NOT_NULL(jobsel_strerror((jobsel_err_t)99));
}
/*
* parse_jobid() (task 1.2, Requirements 5.7, 5.14, 5.15): built on
* parse_jobsel(). BARE -> the exact ID, SINGLE [k] -> the element ID;
* a range, any rejected selection, or a NULL argument -> -EINVAL with
* *jobid left at 0.
*/
static void test_parse_jobid_accepts(void)
{
uint64_t id;
id = 0xdead;
TEST_ASSERT_EQUAL_INT(1, parse_jobid(&id, X_TXT));
TEST_ASSERT_EQUAL_UINT64(X_KEY, id);
id = 0xdead;
TEST_ASSERT_EQUAL_INT(1, parse_jobid(&id, "3-5-1700000000-65541"));
TEST_ASSERT_EQUAL_UINT64(MAKE_JOBID(3, 5, 1700000000u, 0x0, 65541), id);
id = 0xdead;
TEST_ASSERT_EQUAL_INT(1, parse_jobid(&id, X_TXT "[3]"));
TEST_ASSERT_EQUAL_UINT64(
MAKE_JOBID(3, 5, 1700000000u, TAG_JOB_ARRAY, 65536 | 3), id);
TEST_ASSERT_TRUE(is_array_tag(id));
id = 0xdead;
TEST_ASSERT_EQUAL_INT(1, parse_jobid(&id, X_TXT "[32767]"));
TEST_ASSERT_EQUAL_UINT64(
MAKE_JOBID(3, 5, 1700000000u, TAG_JOB_ARRAY, 65536 | 32767), id);
/* surrounding blanks are ignored, as before */
id = 0xdead;
TEST_ASSERT_EQUAL_INT(1, parse_jobid(&id, " \t" X_TXT "[2] \t"));
TEST_ASSERT_EQUAL_UINT64(
MAKE_JOBID(3, 5, 1700000000u, TAG_JOB_ARRAY, 65536 | 2), id);
/* widest identity */
id = 0xdead;
TEST_ASSERT_EQUAL_INT(1, parse_jobid(&id, "63-15-4294967295-1048575"));
TEST_ASSERT_EQUAL_UINT64(MAKE_JOBID(63, 15, 4294967295u, 0x0, 1048575), id);
}
static void test_parse_jobid_rejects(void)
{
/* clang-format off */
static const char* const bad[] = {
X_TXT "[1-3]", /* range */
X_TXT "[3-3]", /* range, single-valued */
X_TXT "[32768]", /* 5.7: index above 32767 */
X_TXT "[99999999999999999999999]",/* 5.7: saturating index */
"3-5-1700000000-5[1]", /* 5.8: misaligned array id */
"64-0-1700000000-5", /* 5.14: cluster too wide */
"3-16-1700000000-5", /* 5.14: apiserver too wide */
"3-5-4294967296-5", /* 5.14: timestamp too wide */
"3-5-1700000000-1048576", /* 5.14: sequence too wide */
"3-5-99999999999999999999999-5", /* 5.14: oversized field */
X_TXT "[1]x", /* 5.15 */
X_TXT "x", /* 5.15 */
"x" X_TXT, /* 5.15 */
"3-5-1700000000", /* 5.15 */
"-3-5-1700000000-5", /* 5.15 */
"+3-5-1700000000-5", /* 5.15 */
X_TXT "[]", /* 5.15 */
X_TXT " [1]", /* 5.15 */
"12345", /* 5.15: bare decimal */
"", /* 5.15 */
};
/* clang-format on */
size_t i;
for (i = 0; i < sizeof(bad) / sizeof(bad[0]); i++) {
uint64_t id = 0xdead;
char msg[160];
snprintf(msg, sizeof(msg), "row %zu: \"%s\"", i, bad[i]);
TEST_ASSERT_EQUAL_INT_MESSAGE(-EINVAL, parse_jobid(&id, (char*)bad[i]),
msg);
TEST_ASSERT_EQUAL_UINT64_MESSAGE(0, id, msg);
}
}
static void test_parse_jobid_null(void)
{
uint64_t id = 0xdead;
TEST_ASSERT_EQUAL_INT(-EINVAL, parse_jobid(NULL, X_TXT));
TEST_ASSERT_EQUAL_INT(-EINVAL, parse_jobid(NULL, NULL));
TEST_ASSERT_EQUAL_INT(-EINVAL, parse_jobid(&id, NULL));
TEST_ASSERT_EQUAL_UINT64(0, id);
}
void test_jobsel(void)
{
RUN_TEST(test_jobsel_selection_cases);
@ -267,4 +359,7 @@ void test_jobsel(void)
RUN_TEST(test_jobsel_is_array_tag);
RUN_TEST(test_jobsel_element_id);
RUN_TEST(test_jobsel_strerror);
RUN_TEST(test_parse_jobid_accepts);
RUN_TEST(test_parse_jobid_rejects);
RUN_TEST(test_parse_jobid_null);
}

74
src/lib/utils.c
View file

@ -16,7 +16,6 @@
#include <execinfo.h>
#endif
#include <regex.h>
#include <tlog.h>
#include "config.h"
@ -618,72 +617,37 @@ int fmt_jobid(char* buf, size_t sz, uint64_t jobid)
return fmt_jobid_ex(buf, sz, jobid, "[", "]");
}
/*
* Parse one written job identity into a job ID, on top of parse_jobsel():
* a bare identity gives the exact ID, `<array-id>[k]` gives the element ID.
* A range, any selection parse_jobsel() rejects (5.7, 5.8, 5.14, 5.15), or a
* NULL argument returns -EINVAL with *jobid (when non-NULL) set to 0.
* Returns 1 on success.
*/
int parse_jobid(uint64_t* jobid, char* buf)
{
regex_t regex;
regmatch_t pmatch[N_MAX_FIELDS_JOBID + 1];
int rc;
int len = strlen(buf);
jobsel_t sel;
if ((jobid == NULL) || (buf == NULL)) {
if (jobid == NULL) {
return -EINVAL;
}
rc = regcomp(&regex, REGEX_JOBARRAY_ID, REG_EXTENDED);
if (rc != 0) {
return -errno;
}
*jobid = 0;
rc = regexec(&regex, buf, N_EL_ARRAY(pmatch), pmatch, 0);
if (rc == 0) {
regfree(&regex);
if ((pmatch[0].rm_so == 0) && (pmatch[0].rm_eo == len)) {
uint32_t tag, id_cluster, id_apiserver, id_timestamp, id_job,
id_idx;
tag = 0x01; /* job array */
id_cluster = atoi(&buf[pmatch[1].rm_so]);
id_apiserver = atoi(&buf[pmatch[2].rm_so]);
id_timestamp = atoi(&buf[pmatch[3].rm_so]);
id_job = atoi(&buf[pmatch[4].rm_so]);
id_idx = atoi(&buf[pmatch[5].rm_so]);
*jobid = MAKE_JOBID(id_cluster, id_apiserver, id_timestamp, tag,
id_job + id_idx);
return 1;
}
if (parse_jobsel(buf, &sel) != JOBSEL_OK) {
return -EINVAL;
}
regfree(&regex);
rc = regcomp(&regex, REGEX_JOBID, REG_EXTENDED);
if (rc != 0) {
return -errno;
}
rc = regexec(&regex, buf, N_EL_ARRAY(pmatch), pmatch, 0);
if (rc == 0) {
uint32_t id_cluster, id_apiserver, id_timestamp, id_job;
if ((pmatch[0].rm_so != 0) || (pmatch[0].rm_eo != len)) {
regfree(&regex);
return -EINVAL;
}
id_cluster = atoi(&buf[pmatch[1].rm_so]);
id_apiserver = atoi(&buf[pmatch[2].rm_so]);
id_timestamp = atoi(&buf[pmatch[3].rm_so]);
id_job = atoi(&buf[pmatch[4].rm_so]);
*jobid
= MAKE_JOBID(id_cluster, id_apiserver, id_timestamp, 0x0, id_job);
regfree(&regex);
switch (sel.form) {
case JOBSEL_BARE:
*jobid = sel.key;
return 1;
case JOBSEL_SINGLE:
*jobid = jobsel_element_id(&sel, sel.lo);
return 1;
case JOBSEL_RANGE:
break;
}
regfree(&regex);
return -EINVAL;
}