Skip to content

Add a nullptr check for char* and const char* #737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@
- When `add_metadata_to_multi_line_logs` in the `PatternFormatter` was set to false, fixed a bug where the last
character of the log message was dropped and added protection for empty messages.
- Updated `LOG_EVERY_N` macros to log on the first occurrence (0th call) instead of waiting until the Nth call.
- Added a `nullptr` check for `char*` and `const char*` during encoding, ensuring the library handles `nullptr` values
gracefully ([#735](https://github.com/odygrd/quill/discussions/735))
- The `CsvWriter` could previously be used with `RotatingFileSink` via the constructor that accepted
`std::shared_ptr<Sink>`, but rotated files did not include the CSV header.
This has now been improved—when using the new constructor that accepts `quill::RotatingFileSinkConfig`,
Expand All @@ -202,7 +204,7 @@
csv_writer.append_row(132121122 + i, "AAPL", i, 100.1, "BUY");
}
```
- When `CsvWriter` is used with `open_mode == 'a'` it won't rewrite the header when the file already exists
- When using `CsvWriter` with `open_mode == 'a'`, the header will no longer be rewritten if the file already exists.
- On Linux, setting a long backend thread name now truncates it instead of
failing. ([#691](https://github.com/odygrd/quill/issues/691))
- Fixed BSD builds. ([#688](https://github.com/odygrd/quill/issues/688))
Expand Down Expand Up @@ -230,18 +232,18 @@
};
```

- `DeferredFormatCodec` now supports both trivially and non-trivially copyable types:
- For trivially copyable types, it behaves the same as `TriviallyCopyableTypeCodec`.
- For non-trivially copyable types, it works similarly to pre-`v4` by taking a copy of the object using the copy
constructor and placement new.
- `DirectFormatCodec` formats the object immediately in the hot path, serving as a shortcut to explicitly formatting
the object when logging.
- For advanced use cases, a custom `Codec` can still be defined for finer control over encoding/decoding.
- `DeferredFormatCodec` now supports both trivially and non-trivially copyable types:
- For trivially copyable types, it behaves the same as `TriviallyCopyableTypeCodec`.
- For non-trivially copyable types, it works similarly to pre-`v4` by taking a copy of the object using the copy
constructor and placement new.
- `DirectFormatCodec` formats the object immediately in the hot path, serving as a shortcut to explicitly formatting
the object when logging.
- For advanced use cases, a custom `Codec` can still be defined for finer control over encoding/decoding.

See:
- [DeferredFormatCodec Usage](https://github.com/odygrd/quill/blob/master/examples/user_defined_types_logging_deferred_format.cpp)
- [DirectFormatCodec Usage](https://github.com/odygrd/quill/blob/master/examples/user_defined_types_logging_direct_format.cpp)
- [Documentation](https://quillcpp.readthedocs.io/en/latest/cheat_sheet.html#logging-user-defined-types)
- [DeferredFormatCodec Usage](https://github.com/odygrd/quill/blob/master/examples/user_defined_types_logging_deferred_format.cpp)
- [DirectFormatCodec Usage](https://github.com/odygrd/quill/blob/master/examples/user_defined_types_logging_direct_format.cpp)
- [Documentation](https://quillcpp.readthedocs.io/en/latest/cheat_sheet.html#logging-user-defined-types)

- Added codec support for C-style arrays of user-defined types in `std/Array.h`
- Fixed warnings: `-Wimplicit-int-float-conversion`, `-Wfloat-equal`, and `-Wdocumentation`.
Expand Down Expand Up @@ -1142,7 +1144,7 @@ within a distinct namespace, there are no conflicts even if you link your own `l
```

- Fixed a bug in timestamp formatting that occasionally displayed an hour component of 0 as
24. ([#329](https://github.com/odygrd/quill/pull/329))
24. ([#329](https://github.com/odygrd/quill/pull/329))

- Added support for specifying a runtime log level, allowing dynamic log level configuration at runtime.
The new runtime log level feature provides flexibility when needed, with a minor overhead cost.
Expand Down
32 changes: 21 additions & 11 deletions include/quill/core/Codec.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ namespace detail

/** We forward declare these to avoid including Utf8Conv.h **/
QUILL_NODISCARD QUILL_EXPORT QUILL_ATTRIBUTE_USED extern std::string utf8_encode(std::wstring_view str);
QUILL_NODISCARD QUILL_EXPORT QUILL_ATTRIBUTE_USED extern std::string utf8_encode(std::byte const* data, size_t wide_str_len);
QUILL_NODISCARD QUILL_EXPORT QUILL_ATTRIBUTE_USED extern std::string utf8_encode(std::byte const* data,
size_t wide_str_len);

#if defined(__MINGW32__)
#pragma GCC diagnostic pop
Expand Down Expand Up @@ -103,6 +104,8 @@ struct is_std_string<std::basic_string<char, std::char_traits<char>, Allocator>>
};
} // namespace detail

static constexpr std::string_view null_c_string{"nullptr"};

/** typename = void for specializations with enable_if **/
template <typename Arg, typename = void>
struct Codec
Expand All @@ -124,10 +127,18 @@ struct Codec
}
else if constexpr (std::disjunction_v<std::is_same<Arg, char*>, std::is_same<Arg, char const*>>)
{
#ifndef NDEBUG
if (arg)
{
assert(((strlen(arg) + 1u) <= std::numeric_limits<uint32_t>::max()) &&
"len is outside the supported range");
}
#endif

// for c strings we do an additional check for nullptr
// include one extra for the zero termination
assert(((strlen(arg) + 1u) <= std::numeric_limits<uint32_t>::max()) &&
"len is outside the supported range");
return conditional_arg_size_cache.push_back(static_cast<uint32_t>(strlen(arg) + 1u));
auto const len = static_cast<uint32_t>(arg ? strlen(arg) : null_c_string.size());
return conditional_arg_size_cache.push_back(len + 1u);
}
else if constexpr (std::disjunction_v<detail::is_std_string<Arg>, std::is_same<Arg, std::string_view>>)
{
Expand Down Expand Up @@ -178,7 +189,8 @@ struct Codec
{
// null terminator is included in the len for c style strings
uint32_t const len = conditional_arg_size_cache[conditional_arg_size_cache_index++];
std::memcpy(buffer, arg, len);
// for c strings we do an additional check for nullptr
std::memcpy(buffer, arg ? arg : null_c_string.data(), len);
buffer += len;
}
else if constexpr (std::disjunction_v<detail::is_std_string<Arg>, std::is_same<Arg, std::string_view>>)
Expand Down Expand Up @@ -284,10 +296,9 @@ template <typename... Args>
QUILL_NODISCARD QUILL_ATTRIBUTE_HOT size_t compute_encoded_size_and_cache_string_lengths(
QUILL_MAYBE_UNUSED SizeCacheVector& conditional_arg_size_cache, Args const&... args)
{
if constexpr (!std::conjunction_v<std::disjunction<
std::is_arithmetic<remove_cvref_t<Args>>, std::is_enum<remove_cvref_t<Args>>,
std::is_same<remove_cvref_t<Args>, void const*>, is_std_string<remove_cvref_t<Args>>,
std::is_same<remove_cvref_t<Args>, std::string_view>>...>)
if constexpr (!std::conjunction_v<std::disjunction<std::is_arithmetic<remove_cvref_t<Args>>, std::is_enum<remove_cvref_t<Args>>,
std::is_same<remove_cvref_t<Args>, void const*>, is_std_string<remove_cvref_t<Args>>,
std::is_same<remove_cvref_t<Args>, std::string_view>>...>)
{
// Clear the cache whenever processing involves non-fundamental types,
// or when the arguments are not of type std::string or std::string_view.
Expand All @@ -313,8 +324,7 @@ QUILL_ATTRIBUTE_HOT void encode(std::byte*& buffer, SizeCacheVector const& condi
Args const&... args)
{
QUILL_MAYBE_UNUSED uint32_t conditional_arg_size_cache_index{0};
(Codec<remove_cvref_t<Args>>::encode(buffer, conditional_arg_size_cache,
conditional_arg_size_cache_index, args),
(Codec<remove_cvref_t<Args>>::encode(buffer, conditional_arg_size_cache, conditional_arg_size_cache_index, args),
...);
}

Expand Down
6 changes: 6 additions & 0 deletions test/integration_tests/StdArrayLoggingTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ TEST_CASE("std_array_logging")
std::array<char const*, 4> scva = {"c style", "string test", "test", "log"};
LOG_INFO(logger, "scva {}", scva);

std::array<char const*, 4> scvan = {"c style", "nullptr", "nullptr", "log"};
LOG_INFO(logger, "scvan {}", scvan);

std::array<std::array<int, 2>, 3> aai = {{{321, 123}, {444, 333}, {111, 222}}};
LOG_INFO(logger, "aai {}", aai);

Expand Down Expand Up @@ -234,6 +237,9 @@ TEST_CASE("std_array_logging")
REQUIRE(quill::testing::file_contains(
file_contents, std::string{"LOG_INFO " + logger_name + " scva [\"c style\", \"string test\", \"test\", \"log\"]"}));

REQUIRE(quill::testing::file_contains(
file_contents, std::string{"LOG_INFO " + logger_name + " scvan [\"c style\", \"nullptr\", \"nullptr\", \"log\"]"}));

REQUIRE(quill::testing::file_contains(
file_contents, std::string{"LOG_INFO " + logger_name + " aai [[321, 123], [444, 333], [111, 222]]"}));

Expand Down
7 changes: 6 additions & 1 deletion test/integration_tests/StringLoggingTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ TEST_CASE("string_logging")

char const* c_style_string_empty = "";
const char* c_style_string = "Lorem ipsum";
char const* csn = nullptr;

char c_style_char_array_empty[] = "";
char const c_style_char_array[] = "dolor";
Expand All @@ -106,6 +107,7 @@ TEST_CASE("string_logging")
const char* npcs = "Example\u0003String\u0004";
LOG_INFO(logger, "non printable cs [{}]", npcs);

LOG_INFO(logger, "csn [{}]", csn);
LOG_INFO(logger, "st [{}]", st);
LOG_INFO(logger, "cas [{}]", cas);
LOG_INFO(logger, "s [{}]", s);
Expand Down Expand Up @@ -160,7 +162,10 @@ TEST_CASE("string_logging")

// Read file and check
std::vector<std::string> const file_contents = quill::testing::file_contents(filename);
REQUIRE_EQ(file_contents.size(), number_of_messages + 19);
REQUIRE_EQ(file_contents.size(), number_of_messages + 20);

REQUIRE(quill::testing::file_contains(
file_contents, std::string{"LOG_INFO " + logger_name + " csn [nullptr]"}));

REQUIRE(quill::testing::file_contains(
file_contents, std::string{"LOG_INFO " + logger_name + " right aligned"}));
Expand Down