lg2: compile time header format checking
Add support to check the format of message headers at compile
time to verify the rules imposed by journald APIs.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I99fcbd8918bbf11ba839843ac8e793d8402b9055
diff --git a/lib/include/phosphor-logging/lg2.hpp b/lib/include/phosphor-logging/lg2.hpp
index d6b8407..6865a31 100644
--- a/lib/include/phosphor-logging/lg2.hpp
+++ b/lib/include/phosphor-logging/lg2.hpp
@@ -7,6 +7,7 @@
#include <phosphor-logging/lg2/concepts.hpp>
#include <phosphor-logging/lg2/conversion.hpp>
#include <phosphor-logging/lg2/flags.hpp>
+#include <phosphor-logging/lg2/header.hpp>
#include <phosphor-logging/lg2/level.hpp>
#include <source_location>
#include <string_view>
@@ -24,9 +25,11 @@
* @param[in] ts - The rest of the arguments.
*/
explicit log(const std::source_location& s, const std::string_view& msg,
- Ts&&... ts)
+ details::header_str_conversion_t<Ts&&>... ts)
{
- details::log_conversion::start(S, s, msg, std::forward<Ts>(ts)...);
+ details::log_conversion::start(
+ S, s, msg,
+ std::forward<details::header_str_conversion_t<Ts&&>>(ts)...);
}
/** default log (source_location is determined by calling location).
@@ -36,9 +39,10 @@
* @param[in] s - The derived source_location.
*/
explicit log(
- const std::string_view& msg, Ts&&... ts,
+ const std::string_view& msg,
+ details::header_str_conversion_t<Ts&&>... ts,
const std::source_location& s = std::source_location::current()) :
- log(s, msg, std::forward<Ts>(ts)...)
+ log(s, msg, std::forward<details::header_str_conversion_t<Ts&&>>(ts)...)
{
}
diff --git a/lib/include/phosphor-logging/lg2/concepts.hpp b/lib/include/phosphor-logging/lg2/concepts.hpp
index 7d997ad..eaea861 100644
--- a/lib/include/phosphor-logging/lg2/concepts.hpp
+++ b/lib/include/phosphor-logging/lg2/concepts.hpp
@@ -1,6 +1,7 @@
#pragma once
#include <concepts>
+#include <type_traits>
namespace lg2::details
{
@@ -9,4 +10,13 @@
template <typename T, typename... Ss>
concept any_but = (... && !std::convertible_to<T, Ss>);
+/** Determine if a type might be a constexpr string: (const char (&)[N]) */
+template <typename T>
+concept maybe_constexpr_string = std::is_array_v<std::remove_cvref_t<T>>&&
+ std::same_as<const char*, std::decay_t<T>>;
+
+/** Determine if a type is certainly not a constexpr string. */
+template <typename T>
+concept not_constexpr_string = !maybe_constexpr_string<T>;
+
}; // namespace lg2::details
diff --git a/lib/include/phosphor-logging/lg2/conversion.hpp b/lib/include/phosphor-logging/lg2/conversion.hpp
index b8e0633..6b51e19 100644
--- a/lib/include/phosphor-logging/lg2/conversion.hpp
+++ b/lib/include/phosphor-logging/lg2/conversion.hpp
@@ -3,6 +3,7 @@
#include <concepts>
#include <cstddef>
#include <phosphor-logging/lg2/flags.hpp>
+#include <phosphor-logging/lg2/header.hpp>
#include <phosphor-logging/lg2/level.hpp>
#include <phosphor-logging/lg2/logger.hpp>
#include <sdbusplus/message/native_types.hpp>
@@ -280,11 +281,14 @@
}
/** Handle conversion of a { Header, Flags, Value } argument set. */
- template <typename... Ts, std::convertible_to<const char*> H,
- log_flags... Fs, typename V, typename... Ss>
- static void step(std::tuple<Ts...>&& ts, H&& h, log_flag<Fs...> f, V&& v,
- Ss&&... ss)
+ template <typename... Ts, log_flags... Fs, typename V, typename... Ss>
+ static void step(std::tuple<Ts...>&& ts, const header_str& h,
+ log_flag<Fs...> f, V&& v, Ss&&... ss)
{
+ static_assert(!std::is_same_v<header_str, std::decay_t<V>>,
+ "Found header_str as value; suggest using std::string to "
+ "avoid unintended conversion.");
+
// These two if conditions are similar, except that one calls 'done'
// since Ss is empty and the other calls the next 'step'.
@@ -295,22 +299,24 @@
if constexpr (sizeof...(Ss) == 0)
{
- apply_done(std::tuple_cat(std::move(ts),
- log_convert(std::forward<H>(h), f, v)));
+ apply_done(
+ std::tuple_cat(std::move(ts), log_convert(h.data(), f, v)));
}
else
{
- step(std::tuple_cat(std::move(ts),
- log_convert(std::forward<H>(h), f, v)),
+ step(std::tuple_cat(std::move(ts), log_convert(h.data(), f, v)),
std::forward<Ss>(ss)...);
}
}
/** Handle conversion of a { Header, Value } argument set. */
- template <typename... Ts, std::convertible_to<const char*> H, typename V,
- typename... Ss>
- static void step(std::tuple<Ts...>&& ts, H&& h, V&& v, Ss&&... ss)
+ template <typename... Ts, typename V, typename... Ss>
+ static void step(std::tuple<Ts...>&& ts, const header_str& h, V&& v,
+ Ss&&... ss)
{
+ static_assert(!std::is_same_v<header_str, std::decay_t<V>>,
+ "Found header_str as value; suggest using std::string to "
+ "avoid unintended conversion.");
// These two if conditions are similar, except that one calls 'done'
// since Ss is empty and the other calls the next 'step'.
@@ -321,33 +327,32 @@
if constexpr (sizeof...(Ss) == 0)
{
- apply_done(
- std::tuple_cat(std::move(ts), log_convert(std::forward<H>(h),
- log_flag<>{}, v)));
+ apply_done(std::tuple_cat(std::move(ts),
+ log_convert(h.data(), log_flag<>{}, v)));
}
else
{
- step(std::tuple_cat(std::move(ts), log_convert(std::forward<H>(h),
- log_flag<>{}, v)),
+ step(std::tuple_cat(std::move(ts),
+ log_convert(h.data(), log_flag<>{}, v)),
std::forward<Ss>(ss)...);
}
}
/** Finding a non-string as the first argument of a 2 or 3 argument set
* is an error (missing HEADER field). */
- template <typename... Ts, any_but<const char*> H, typename... Ss>
+ template <typename... Ts, not_constexpr_string H, typename... Ss>
static void step(std::tuple<Ts...>&&, H, Ss&&...)
{
- static_assert(std::is_same_v<const char*, H>,
+ static_assert(std::is_same_v<header_str, H>,
"Found value without expected header field.");
}
/** Finding a free string at the end is an error (found HEADER but no data).
*/
- template <typename... Ts, std::convertible_to<const char*> H>
- static void step(std::tuple<Ts...>&&, H)
+ template <typename... Ts>
+ static void step(std::tuple<Ts...>&&, header_str)
{
- static_assert(!std::is_convertible_v<const char*, H>,
+ static_assert(!std::is_same_v<header_str, header_str>,
"Found header field without expected data.");
}
diff --git a/lib/include/phosphor-logging/lg2/header.hpp b/lib/include/phosphor-logging/lg2/header.hpp
new file mode 100644
index 0000000..f767427
--- /dev/null
+++ b/lib/include/phosphor-logging/lg2/header.hpp
@@ -0,0 +1,76 @@
+#pragma once
+
+#include <phosphor-logging/lg2/concepts.hpp>
+#include <string_view>
+
+namespace lg2::details
+{
+
+/** A type to handle compile-time validation of header strings. */
+struct header_str
+{
+ // Hold the header string value.
+ std::string_view value;
+
+ /** Constructor which performs validation. */
+ template <typename T>
+ consteval header_str(const T& s) : value(s)
+ {
+ if (value.size() == 0)
+ {
+ report_error(
+ "journald requires headers must have non-zero length.");
+ }
+ if (value[0] == '_')
+ {
+ report_error("journald requires header do not start with "
+ "underscore (_)");
+ }
+
+ if (value.find_first_not_of("ABCDEFGHIJKLMNOPQRSTUVWXYZ_0123456789") !=
+ std::string_view::npos)
+ {
+ report_error(
+ "journald requires header may only contain underscore, "
+ "uppercase letters, or numbers ([_A-Z0-9]).");
+ }
+ }
+
+ /** Cast conversion back to (const char*). */
+ operator const char*() const
+ {
+ return value.data();
+ }
+
+ const char* data() const
+ {
+ return value.data();
+ }
+
+ private:
+ // This does nothing, but is useful for creating nice compile errors in
+ // a constexpr context.
+ static void report_error(const char*);
+};
+
+/** A helper type for constexpr conversion into header_str, if
+ * 'maybe_constexpr_string'. For non-constexpr string, this does nothing.
+ */
+template <typename T>
+struct header_str_conversion
+{
+ using type = T;
+};
+
+/** Specialization for maybe_constexpr_string. */
+template <maybe_constexpr_string T>
+struct header_str_conversion<T>
+{
+ using type = const header_str&;
+};
+
+/** std-style _t alias for header_str_conversion. */
+template <typename T>
+using header_str_conversion_t = typename header_str_conversion<T>::type;
+
+} // namespace lg2::details
diff --git a/lib/include/phosphor-logging/meson.build b/lib/include/phosphor-logging/meson.build
index ebe52ac..2b166cb 100644
--- a/lib/include/phosphor-logging/meson.build
+++ b/lib/include/phosphor-logging/meson.build
@@ -26,6 +26,7 @@
'lg2/concepts.hpp',
'lg2/conversion.hpp',
'lg2/flags.hpp',
+ 'lg2/header.hpp',
'lg2/level.hpp',
'lg2/logger.hpp',
subdir: 'phosphor-logging/lg2',