Skip to content

Commit

Permalink
snprintf truncate behavior by default (#256)
Browse files Browse the repository at this point in the history
* wip

* snprintf behavior always

* restore formatting

* remove older strategy change

* fix bug, test, golf
  • Loading branch information
charlesnicholson authored Jan 13, 2024
1 parent 9943d54 commit 64333b3
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 58 deletions.
1 change: 0 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ set(unit_test_files
tests/unit_fsplit_abs.cc
tests/unit_snprintf.cc
tests/unit_snprintf_safe_empty.cc
tests/unit_snprintf_safe_trim.cc
tests/unit_vpprintf.cc)

npf_test(unit_tests_normal_sized_formatters "${unit_test_files}")
Expand Down
9 changes: 3 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,11 @@ If no configuration flags are specified, nanoprintf will default to "reasonable"
If a disabled format specifier feature is used, no conversion will occur and the format specifier string simply will be printed instead.

### Sprintf Safety
By default, npf_snprintf and npf_vsnprintf behave according to the C Standard: the provided buffer will be filled but not overrun, though a null-terminator `0` byte will *not* be written at the end if the buffer is exhausted!
By default, npf_snprintf and npf_vsnprintf behave according to the C Standard: the provided buffer will be filled but not overrun. If the string would have overrun the buffer, a null-terminator byte will be written to the final byte of the buffer. If the buffer is `null` or zero-sized, no bytes will be written.

nanoprintf offers three options for configuring safety:
* Do nothing. User-provided buffers will not be null-terminated if exhausted.
* Define `NANOPRINTF_SNPRINTF_SAFE_TRIM_STRING_ON_OVERFLOW`: When exhausted, the final byte of the buffer will be overwritten with a null-terimator byte. This is similar in spirit to the behavior of [BSD strlcpy](https://linux.die.net/man/3/strlcpy).
* Define `NANOPRINTF_SNPRINTF_SAFE_EMPTY_STRING_ON_OVERFLOW`: When exhausted, the _first_ byte of the buffer will be overwritten with a null-terminator byte. This is similar in spirit to [Microsoft's snprintf_s](https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-s-snprintf-s-l-snwprintf-s-snwprintf-s-l).
If you define `NANOPRINTF_SNPRINTF_SAFE_EMPTY_STRING_ON_OVERFLOW` and your string is larger than your buffer, the _first_ byte of the buffer will be overwritten with a null-terminator byte. This is similar in spirit to [Microsoft's snprintf_s](https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-s-snprintf-s-l-snwprintf-s-snwprintf-s-l).

In any of the above cases, nanoprintf will still return the number of bytes that would have been written to the buffer, had there been enough room. This value does not account for the null-terminator byte, in accordance with the C Standard.
In all cases, nanoprintf will return the number of bytes that would have been written to the buffer, had there been enough room. This value does not account for the null-terminator byte, in accordance with the C Standard.

### Thread Safety
nanoprintf uses only stack memory and no concurrency primitives, so internally it is oblivious to its execution environment. This makes it safe to call from multiple execution contexts concurrently, or to interrupt a `npf_` call with another `npf_` call (say, an ISR or something). If you use `npf_pprintf` concurrently with the same `npf_putc` target, it's up to you to ensure correctness inside your callback. If you `npf_snprintf` from multiple threads to the same buffer, you will have an obvious data race.
Expand Down
13 changes: 5 additions & 8 deletions nanoprintf.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,6 @@ NPF_VISIBILITY int npf_vpprintf(
#error Precision format specifiers must be enabled if float support is enabled.
#endif

#if defined(NANOPRINTF_SNPRINTF_SAFE_EMPTY_STRING_ON_OVERFLOW) && \
defined(NANOPRINTF_SNPRINTF_SAFE_TRIM_STRING_ON_OVERFLOW)
#error snprintf safety flags are mutually exclusive.
#endif

// intmax_t / uintmax_t require stdint from c99 / c++11
#if NANOPRINTF_USE_LARGE_FORMAT_SPECIFIERS == 1
#ifndef _MSC_VER
Expand Down Expand Up @@ -1045,11 +1040,13 @@ int npf_vsnprintf(char *buffer, size_t bufsz, char const *format, va_list vlist)
int const n = npf_vpprintf(pc, &bufputc_ctx, format, vlist);
pc('\0', &bufputc_ctx);

if (buffer && bufsz) {
#ifdef NANOPRINTF_SNPRINTF_SAFE_EMPTY_STRING_ON_OVERFLOW
if (bufsz && (n >= (int)bufsz)) { buffer[0] = '\0'; }
#elif defined(NANOPRINTF_SNPRINTF_SAFE_TRIM_STRING_ON_OVERFLOW)
if (bufsz && (n >= (int)bufsz)) { buffer[bufsz - 1] = '\0'; }
if (n >= (int)bufsz) { buffer[0] = '\0'; }
#else
buffer[bufsz - 1] = '\0';
#endif
}

return n;
}
Expand Down
53 changes: 50 additions & 3 deletions tests/unit_snprintf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,66 @@ TEST_CASE("npf_snprintf") {
}

SUBCASE("fills buffer fully") {
REQUIRE(npf_snprintf(buf, 4, "abcd") == 4);
REQUIRE(npf_snprintf(buf, 5, "abcd") == 4);
REQUIRE(buf[0] == 'a');
REQUIRE(buf[1] == 'b');
REQUIRE(buf[2] == 'c');
REQUIRE(buf[3] == 'd');
REQUIRE(buf[4] == '\0');
}

SUBCASE("last byte is null terminator") {
REQUIRE(npf_snprintf(buf, 4, "abcd") == 4);
REQUIRE(buf[0] == 'a');
REQUIRE(buf[1] == 'b');
REQUIRE(buf[2] == 'c');
REQUIRE(buf[3] == '\0');
}

SUBCASE("doesn't write past end of buffer") {
SUBCASE("terminates before end of buffer") {
buf[3] = '*';
REQUIRE(npf_snprintf(buf, 3, "abcd") == 4);
REQUIRE(buf[0] == 'a');
REQUIRE(buf[1] == 'b');
REQUIRE(buf[2] == 'c');
REQUIRE(buf[2] == '\0');
REQUIRE(buf[3] == '*');
}

SUBCASE("string trimming") {
buf[0] = '@';
buf[7] = '*';
buf[8] = '!';

SUBCASE("zero-sized buffer") {
REQUIRE(npf_snprintf(buf, 0, "abc") == 3);
REQUIRE(buf[0] == '@');
}

SUBCASE("small string") {
REQUIRE(npf_snprintf(buf, 8, "abc") == 3);
REQUIRE(std::string{buf} == "abc");
}

SUBCASE("exact fit string") {
REQUIRE(npf_snprintf(buf, 8, "1234567") == 7);
REQUIRE(buf[7] == '\0');
REQUIRE(std::string{buf} == "1234567");
}

SUBCASE("if the null terminator doesn't fit, the string is trimmed") {
REQUIRE(npf_snprintf(buf, 8, "12345678") == 8);
REQUIRE(std::string{buf} == "1234567");
REQUIRE(buf[8] == '!');
}

SUBCASE("if the string contents are too long, the string is trimmed") {
REQUIRE(npf_snprintf(buf, 8, "123456789") == 9);
REQUIRE(std::string{buf} == "1234567");
REQUIRE(buf[8] == '!');
}

SUBCASE("null buffer with non-null length doesn't get terminated") {
npf_snprintf(nullptr, 4, "abcd");
}
}
}
40 changes: 0 additions & 40 deletions tests/unit_snprintf_safe_trim.cc

This file was deleted.

0 comments on commit 64333b3

Please sign in to comment.