diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 0218803..87ee98e 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -92,8 +92,8 @@ jobs: # there is no lcov/gcovr to install here. # # The gate is a ratchet, not a target. Across all four sources the suite - # covers 99.5% of lines (1643/1651), 46.0% of branches and 100% of - # functions (147/147), so 90/40 fails on a real regression -- a test + # covers 99.5% of lines (1708/1716), 46.0% of branches and 100% of + # functions (154/154), so 90/40 fails on a real regression -- a test # deleted, or new untested code added -- without tripping over rounding. # The report is printed either way; the uncovered lines it lists are the # missing tests. diff --git a/README.md b/README.md index 9c1f79e..59c0f9e 100644 --- a/README.md +++ b/README.md @@ -360,10 +360,10 @@ cmake -S . -B build-coverage -DAKSL_COVERAGE=ON \ | file | lines | branches | functions | |---|---|---|---| | `src/collections.c` | 99.3% (601/605) | 43.5% | 100% (43/43) | -| `src/stdlib.c` | 99.3% (557/561) | 46.5% | 100% (50/50) | -| `src/stream.c` | 100% (274/274) | 43.4% | 100% (31/31) | +| `src/stdlib.c` | 99.4% (614/618) | 46.5% | 100% (55/55) | +| `src/stream.c` | 100% (282/282) | 43.4% | 100% (33/33) | | `src/string.c` | 100% (211/211) | 53.8% | 100% (23/23) | -| **total** | **99.5% (1643/1651)** | **46.0%** | **100% (147/147)** | +| **total** | **99.5% (1708/1716)** | **46.0%** | **100% (154/154)** | so the 90/40 gate above is a ratchet with headroom rather than a target. Eight lines are uncovered and each is uncovered on purpose: diff --git a/TODO.md b/TODO.md index 56986e8..f89b9db 100644 --- a/TODO.md +++ b/TODO.md @@ -11,11 +11,11 @@ second, what is missing last. | | | |---|---| -| Wrapped | 147 public functions across `src/stdlib.c`, `src/string.c`, `src/stream.c`, `src/collections.c` | +| Wrapped | 154 public functions across `src/stdlib.c`, `src/string.c`, `src/stream.c`, `src/collections.c` | | Tests | 17 CTest binaries plus 2 negative-compile entries, green under the default and sanitizer builds | -| Line coverage | 99.5% (1643/1651) | -| Function coverage | 100% (147/147) | -| Doxygen | 100%, gated — `cmake --build build --target docs` fails on an undocumented function, parameter or return | +| Line coverage | 99.5% (1708/1716) | +| Function coverage | 100% (154/154) | +| Doxygen | 100% of 154, gated — `cmake --build build --target docs` fails on an undocumented function, parameter or return | The six confirmed defects that used to head this file are fixed and `AKSL_KNOWN_FAILING_TESTS` is empty. What they were, and what changed as a diff --git a/src/stdlib.c b/src/stdlib.c index ca16ccc..db882bb 100644 --- a/src/stdlib.c +++ b/src/stdlib.c @@ -541,30 +541,33 @@ akerr_ErrorContext AKERR_NOIGNORE *aksl_vasprintf(int *count, char **dest, const *dest = NULL; FAIL_ZERO_RETURN(e, format, AKERR_NULLPOINTER, "count=%p, dest=%p, format=%p", (void *)count, (void *)dest, (void *)format); + /* + * Measured with a copy, because a vsnprintf consumes the va_list and using + * the same one twice is undefined behaviour rather than merely wrong. + */ va_copy(measure, args); needed = vsnprintf(NULL, 0, format, measure); va_end(measure); FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "could not format the arguments"); ATTEMPT { CATCH(e, aksl_malloc((size_t)needed + 1, (void **)&buf)); - /* - * Cannot truncate -- the buffer was sized from this same format and - * arguments -- but the return is checked anyway, because "cannot happen" - * is a claim about the line above rather than about vsnprintf. - */ - FAIL_NONZERO_BREAK(e, (vsnprintf(buf, (size_t)needed + 1, format, args) != needed), - AKERR_IO, "formatted output changed length between passes"); - *dest = buf; - *count = needed; } CLEANUP { - if ( *dest == NULL && buf != NULL ) { - akerr_ErrorContext *released = aksl_free(buf); - if ( released != NULL ) { - released = akerr_release_error(released); - } - } } PROCESS(e) { } FINISH(e, true); + /* + * No second check on the write, and no cleanup path for one. + * + * The buffer was sized by vsnprintf from this same format string and these + * same arguments a few lines up, so the write cannot truncate. Guarding it + * anyway would mean a failure branch nothing can reach and a free() beneath + * it that nothing can execute -- dead code that reads as though there were a + * failure mode to catch, which is exactly what TODO.md 2.2.11 recorded + * against the old aksl_memset and aksl_memcpy and what removing those was + * for. The invariant is stated here instead, where it can be read. + */ + vsnprintf(buf, (size_t)needed + 1, format, args); + *dest = buf; + *count = needed; SUCCEED_RETURN(e); } diff --git a/tests/test_streamio.c b/tests/test_streamio.c index 26e9366..cb090be 100644 --- a/tests/test_streamio.c +++ b/tests/test_streamio.c @@ -567,6 +567,56 @@ static int test_scanf_reads_stdin(void) return 0; } +/* The va_list form, used the way a consumer building its own wrapper would. */ +static akerr_ErrorContext AKERR_NOIGNORE *consumer_scan(const char *fmt, int expected, + int *assigned, ...) +{ + va_list args; + akerr_ErrorContext *raised = NULL; + + va_start(args, assigned); + raised = aksl_vscanf(fmt, expected, assigned, args); + va_end(args); + return raised; +} + +static int test_vscanf_reads_stdin(void) +{ + char path[AKSL_TMP_MAX]; + FILE *fp = NULL; + akerr_ErrorContext *err = NULL; + int a = 0; + int assigned = 0; + int saved = -1; + int restored = -1; + + AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0); + fp = fopen(path, "w"); + AKSL_CHECK(fp != NULL); + AKSL_CHECK(fputs("33\n", fp) != EOF); + AKSL_CHECK(fclose(fp) == 0); + + saved = dup(fileno(stdin)); + AKSL_CHECK(saved >= 0); + if ( freopen(path, "r", stdin) == NULL ) { + close(saved); + AKSL_CHECK(0); + } + + err = consumer_scan("%d", 1, &assigned, &a); + + restored = dup2(saved, fileno(stdin)); + close(saved); + clearerr(stdin); + + AKSL_CHECK(restored >= 0); + AKSL_CHECK(aksl_take(err) == 0); + AKSL_CHECK(assigned == 1); + AKSL_CHECK(a == 33); + AKSL_CHECK(unlink(path) == 0); + return 0; +} + /* ---------------------------------------------------------------------- */ /* Files */ /* ---------------------------------------------------------------------- */ @@ -651,6 +701,7 @@ int main(void) AKSL_RUN(failures, test_fscanf_reads_from_a_stream); AKSL_RUN(failures, test_scanf_reads_stdin); + AKSL_RUN(failures, test_vscanf_reads_stdin); AKSL_RUN(failures, test_remove_and_rename); AKSL_RUN(failures, test_mkstemp_and_mkdtemp);