From 02c40fa7fade319bf764c96c659514ac7bb8bafb Mon Sep 17 00:00:00 2001 From: huntedbytheirs Date: Sat, 29 Aug 2026 01:28:54 -0400 Subject: [PATCH] fix: address F2/F4 review findings (leaks, qsort guard, err contract, compiler-default layering) --- src/detect/probe.c | 1 - src/ext/discovery.c | 11 +++++++++-- src/gen/args.c | 6 +++--- src/kdl/ast.h | 10 +++++++--- src/kdl/parser.c | 11 +++++++++-- src/main.c | 23 ++++++++++++++++++++--- 6 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/detect/probe.c b/src/detect/probe.c index 665a868..1f82e24 100644 --- a/src/detect/probe.c +++ b/src/detect/probe.c @@ -133,7 +133,6 @@ static const char CACHE_FUNCS[] = "# snippets read results via st_cache_get and record them via\n" "# st_cache_set. Cache key: ac_cv__.\n" "\n" - ": \"${CC:=cc}\"\n" ": \"${CFLAGS:=}\"\n" ": \"${st_cc_id:=unknown}\"\n" ": \"${cross_compiling:=}\"\n" diff --git a/src/ext/discovery.c b/src/ext/discovery.c index cdcdea7..d058a7d 100644 --- a/src/ext/discovery.c +++ b/src/ext/discovery.c @@ -167,7 +167,13 @@ read_module_source(const char *path, char **out, size_t *out_len) return st_error_internal("extension load failed: out of memory"); } got = fread(buf, 1, (size_t)size, f); - if (got != (size_t)size || fclose(f) != 0) { + if (got != (size_t)size) { + /* short read: close BEFORE the error return (no leak) */ + (void)fclose(f); + free(buf); + goto io_fail; + } + if (fclose(f) != 0) { free(buf); goto io_fail; } @@ -336,7 +342,8 @@ discover_dir(struct st_lua_rt *rt, const char *dir) free(names); return err; } - qsort(names, n, sizeof *names, cmp_str); + if (n > 1) + qsort(names, n, sizeof *names, cmp_str); for (i = 0; i < n && err == NULL; i++) { char *path; struct stat st; diff --git a/src/gen/args.c b/src/gen/args.c index faf300e..48206f3 100644 --- a/src/gen/args.c +++ b/src/gen/args.c @@ -388,8 +388,8 @@ static const char ARGS_PRE[] = "# Full argument parsing (todo 18): --help/-h, --version, --prefix/\n" "# --exec-prefix/--host/--build in both '=' and space forms, the\n" "# per-option --enable-/--disable- flags, and generic\n" - "# VAR=VALUE overrides (positional, e.g. `./configure CC=clang`, or\n" - "# exported by the invoking shell, e.g. `CC=clang ./configure`).\n" + "# VAR=VALUE overrides (positional, e.g. `./configure VAR=VALUE`, or\n" + "# exported by the invoking shell, e.g. `VAR=VALUE ./configure`).\n" "# Unknown --* options error. The loop only READS \"$@\", so the\n" "# config.log init right after this section still sees the complete\n" "# original invocation.\n" @@ -414,7 +414,7 @@ static const char USAGE_HEAD[] = " printf '%s\\n' ' differs from --build -> cross-compiling'\n" " printf '%s\\n' ' --build=TRIPLET, --build TRIPLET'\n" " printf '%s\\n' ' the platform the build is running on'\n" - " printf '%s\\n' ' VAR=VALUE override the variable VAR (e.g. CC=clang);'\n" + " printf '%s\\n' ' VAR=VALUE override a variable before substitution;'\n" " printf '%s\\n' ' both VAR=VALUE ./configure and'\n" " printf '%s\\n' ' ./configure VAR=VALUE work'\n"; diff --git a/src/kdl/ast.h b/src/kdl/ast.h index 9660e85..0a04485 100644 --- a/src/kdl/ast.h +++ b/src/kdl/ast.h @@ -91,9 +91,13 @@ struct st_kdl_document { /* Parse `src` (a NUL-terminated buffer, borrowed) into an AST. On success * returns the document — possibly with zero nodes — and leaves `*err` - * untouched. On failure returns NULL and, if `err != NULL`, sets `*err` - * to an owned st_error of category ST_ERR_KDL_PARSE with a stable span - * (see the header comment). `filename` is borrowed into every span. */ + * untouched. On failure returns NULL and sets `*err` to an owned st_error + * of category ST_ERR_KDL_PARSE with a stable span (see the header + * comment). `err` is REQUIRED: it must point to a valid + * `struct st_error *` — every caller passes `&err`. NULL is tolerated + * only as a last-resort fallback (the diagnostic is then discarded, and + * the document is still NULL on failure), but no new caller should rely + * on it. `filename` is borrowed into every span. */ struct st_kdl_document *st_kdl_parse(const char *src, const char *filename, struct st_error **err); diff --git a/src/kdl/parser.c b/src/kdl/parser.c index dbaba13..57fb3c7 100644 --- a/src/kdl/parser.c +++ b/src/kdl/parser.c @@ -621,10 +621,14 @@ st_kdl_parse(const char *src, const char *filename, struct st_error **err) struct st_kdl_document *doc; struct st_kdl_node *head = NULL; struct st_kdl_node *tail = NULL; + struct st_error *local_err = NULL; - if (err != NULL) { - *err = NULL; + if (err == NULL) { + /* last-resort fallback: the documented contract requires a + * non-NULL err; a NULL caller discards the diagnostic */ + err = &local_err; } + *err = NULL; parser.lx = st_lexer_new(src, filename); if (parser.lx == NULL) { @@ -684,6 +688,9 @@ fail: st_lexer_free(parser.lx); nodes_free(head); free(doc); + if (err == &local_err && local_err != NULL) { + st_error_free(local_err); + } return NULL; } diff --git a/src/main.c b/src/main.c index 81a4991..7e48a06 100644 --- a/src/main.c +++ b/src/main.c @@ -109,7 +109,21 @@ read_file(const char *path, char **out) return st_error_internal("out of memory"); } got = fread(buf, 1, (size_t)size, f); - if (got != (size_t)size || fclose(f) != 0) { + if (got != (size_t)size) { + /* short read: close BEFORE the error return (no leak) */ + (void)fclose(f); + free(buf); + need = strlen(path) + 64; + buf = malloc(need); + if (buf == NULL) + return st_error_internal("out of memory"); + (void)snprintf(buf, need, "cannot read build file '%s': %s", + path, strerror(errno)); + err = st_error_io(buf); + free(buf); + return err; + } + if (fclose(f) != 0) { free(buf); need = strlen(path) + 64; buf = malloc(need); @@ -205,8 +219,11 @@ main(int argc, char **argv) { const char *cc = st_registry_get_var(st_ext_var_registry(ctx), "CC"); - printf("wrote ./configure (CC=%s)\n", - cc != NULL ? cc : "cc"); + + if (cc != NULL) + printf("wrote ./configure (CC=%s)\n", cc); + else + printf("wrote ./configure\n"); } rc = CLI_EXIT_OK;