diff --git a/CMakeLists.txt b/CMakeLists.txt index 6e47bdb..74fe7ea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -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}") diff --git a/README.md b/README.md index 4845746..7c28968 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/nanoprintf.h b/nanoprintf.h index cec038b..23e4f05 100644 --- a/nanoprintf.h +++ b/nanoprintf.h @@ -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 @@ -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; } diff --git a/tests/unit_snprintf.cc b/tests/unit_snprintf.cc index e0e06f3..74a85ab 100644 --- a/tests/unit_snprintf.cc +++ b/tests/unit_snprintf.cc @@ -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"); + } + } } diff --git a/tests/unit_snprintf_safe_trim.cc b/tests/unit_snprintf_safe_trim.cc deleted file mode 100644 index b5fc4ad..0000000 --- a/tests/unit_snprintf_safe_trim.cc +++ /dev/null @@ -1,40 +0,0 @@ -#define NANOPRINTF_SNPRINTF_SAFE_TRIM_STRING_ON_OVERFLOW -#include "unit_nanoprintf.h" - -#include - -TEST_CASE("snprintf safety: trim string") { - char buf[9]; - 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] == '!'); - } -} -