Test aksl_vscanf, and drop a check that could not fail
Coverage caught both. aksl_vscanf was declared, documented and never called by anything, which is what 100% function coverage is for -- it is the only metric here that notices a whole function nobody exercises. aksl_vasprintf had a second vsnprintf whose return it compared against the first, and a CLEANUP block to free the buffer when they disagreed. They cannot disagree: the buffer was sized by vsnprintf from the same format string and the same arguments a few lines earlier. So that was a failure branch nothing can reach and a free() beneath it that nothing can execute -- exactly the dead code TODO.md 2.2.11 recorded against the old aksl_memset and aksl_memcpy, and removing those was the point. The invariant is a comment now, where it can be read. Back to 99.5% of lines (1708/1716) and 100% of functions (154/154), with the eight uncovered lines the ones TODO.md already accounts for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -92,8 +92,8 @@ jobs:
|
|||||||
# there is no lcov/gcovr to install here.
|
# there is no lcov/gcovr to install here.
|
||||||
#
|
#
|
||||||
# The gate is a ratchet, not a target. Across all four sources the suite
|
# 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
|
# covers 99.5% of lines (1708/1716), 46.0% of branches and 100% of
|
||||||
# functions (147/147), so 90/40 fails on a real regression -- a test
|
# functions (154/154), so 90/40 fails on a real regression -- a test
|
||||||
# deleted, or new untested code added -- without tripping over rounding.
|
# deleted, or new untested code added -- without tripping over rounding.
|
||||||
# The report is printed either way; the uncovered lines it lists are the
|
# The report is printed either way; the uncovered lines it lists are the
|
||||||
# missing tests.
|
# missing tests.
|
||||||
|
|||||||
@@ -360,10 +360,10 @@ cmake -S . -B build-coverage -DAKSL_COVERAGE=ON \
|
|||||||
| file | lines | branches | functions |
|
| file | lines | branches | functions |
|
||||||
|---|---|---|---|
|
|---|---|---|---|
|
||||||
| `src/collections.c` | 99.3% (601/605) | 43.5% | 100% (43/43) |
|
| `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/stdlib.c` | 99.4% (614/618) | 46.5% | 100% (55/55) |
|
||||||
| `src/stream.c` | 100% (274/274) | 43.4% | 100% (31/31) |
|
| `src/stream.c` | 100% (282/282) | 43.4% | 100% (33/33) |
|
||||||
| `src/string.c` | 100% (211/211) | 53.8% | 100% (23/23) |
|
| `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
|
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:
|
lines are uncovered and each is uncovered on purpose:
|
||||||
|
|||||||
8
TODO.md
8
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 |
|
| Tests | 17 CTest binaries plus 2 negative-compile entries, green under the default and sanitizer builds |
|
||||||
| Line coverage | 99.5% (1643/1651) |
|
| Line coverage | 99.5% (1708/1716) |
|
||||||
| Function coverage | 100% (147/147) |
|
| Function coverage | 100% (154/154) |
|
||||||
| Doxygen | 100%, gated — `cmake --build build --target docs` fails on an undocumented function, parameter or return |
|
| 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
|
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
|
`AKSL_KNOWN_FAILING_TESTS` is empty. What they were, and what changed as a
|
||||||
|
|||||||
33
src/stdlib.c
33
src/stdlib.c
@@ -541,30 +541,33 @@ akerr_ErrorContext AKERR_NOIGNORE *aksl_vasprintf(int *count, char **dest, const
|
|||||||
*dest = NULL;
|
*dest = NULL;
|
||||||
FAIL_ZERO_RETURN(e, format, AKERR_NULLPOINTER, "count=%p, dest=%p, format=%p",
|
FAIL_ZERO_RETURN(e, format, AKERR_NULLPOINTER, "count=%p, dest=%p, format=%p",
|
||||||
(void *)count, (void *)dest, (void *)format);
|
(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);
|
va_copy(measure, args);
|
||||||
needed = vsnprintf(NULL, 0, format, measure);
|
needed = vsnprintf(NULL, 0, format, measure);
|
||||||
va_end(measure);
|
va_end(measure);
|
||||||
FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "could not format the arguments");
|
FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "could not format the arguments");
|
||||||
ATTEMPT {
|
ATTEMPT {
|
||||||
CATCH(e, aksl_malloc((size_t)needed + 1, (void **)&buf));
|
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 {
|
} CLEANUP {
|
||||||
if ( *dest == NULL && buf != NULL ) {
|
|
||||||
akerr_ErrorContext *released = aksl_free(buf);
|
|
||||||
if ( released != NULL ) {
|
|
||||||
released = akerr_release_error(released);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} PROCESS(e) {
|
} PROCESS(e) {
|
||||||
} FINISH(e, true);
|
} 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);
|
SUCCEED_RETURN(e);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -567,6 +567,56 @@ static int test_scanf_reads_stdin(void)
|
|||||||
return 0;
|
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 */
|
/* Files */
|
||||||
/* ---------------------------------------------------------------------- */
|
/* ---------------------------------------------------------------------- */
|
||||||
@@ -651,6 +701,7 @@ int main(void)
|
|||||||
AKSL_RUN(failures, test_fscanf_reads_from_a_stream);
|
AKSL_RUN(failures, test_fscanf_reads_from_a_stream);
|
||||||
|
|
||||||
AKSL_RUN(failures, test_scanf_reads_stdin);
|
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_remove_and_rename);
|
||||||
AKSL_RUN(failures, test_mkstemp_and_mkdtemp);
|
AKSL_RUN(failures, test_mkstemp_and_mkdtemp);
|
||||||
|
|||||||
Reference in New Issue
Block a user