Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c21b48db authored by Henri Chataing's avatar Henri Chataing Committed by Gerrit Code Review
Browse files

Merge changes Ie095e56e,I9325f2cb,Ie0fe8cc8 into main 

* changes:
  system/log: Make log::fatal [[noreturn]]
  system/log: Implement source_location helper
  system/log: Implement log::fatal_if helper
parents 033213d6 dcdd0add

system/log/include/bluetooth/log.h

+89 −15
Original line number Diff line number Diff line
@@ -38,11 +38,24 @@ enum Level { 
  kFatal = 7,

};



/// Information about the location a log is printed from.

/// Passing this parameter by default value will fill in

/// the correct information.

struct source_location {

  source_location(char const* file_name = __builtin_FILE(),

                  int line = __builtin_LINE(),

                  char const* function_name = __builtin_FUNCTION())

      : line(line), file_name(file_name), function_name(function_name) {}



  int line;

  char const* file_name;

  char const* function_name;

};



/// Write a single log line.

/// The implementation of this function is dependent on the backend.

void vlog(Level level, char const* tag, char const* file_name, int line,

          char const* function_name, fmt::string_view fmt,

          fmt::format_args vargs);

void vlog(Level level, char const* tag, source_location location,

          fmt::string_view fmt, fmt::format_args vargs);



/// Capture invalid parameter values that would cause runtime

/// formatting errors.
@@ -70,10 +83,8 @@ char*& format_replace(char*& arg) { 
template <Level level, typename... T>

struct log {

  log(fmt::format_string<T...> fmt, T&&... args,

      char const* file_name = __builtin_FILE(), int line = __builtin_LINE(),

      char const* function_name = __builtin_FUNCTION()) {

    vlog(level, LOG_TAG, file_name, line, function_name,

         static_cast<fmt::string_view>(fmt),

      source_location location = source_location()) {

    vlog(level, LOG_TAG, location, static_cast<fmt::string_view>(fmt),

         fmt::make_format_args(format_replace(args)...));

  }

};
@@ -91,8 +102,6 @@ namespace bluetooth::log { 


#if (__cplusplus >= 202002L && defined(__GNUC__) && !defined(__clang__))



template <typename... T>

using fatal = log_internal::log<log_internal::kFatal, T...>;

template <typename... T>

using error = log_internal::log<log_internal::kError, T...>;

template <typename... T>
@@ -106,10 +115,6 @@ using verbose = log_internal::log<log_internal::kVerbose, T...>; 


#else



template <typename... T>

struct fatal : log_internal::log<log_internal::kFatal, T...> {

  using log_internal::log<log_internal::kFatal, T...>::log;

};

template <typename... T>

struct error : log_internal::log<log_internal::kError, T...> {

  using log_internal::log<log_internal::kError, T...>::log;
@@ -131,8 +136,6 @@ struct verbose : log_internal::log<log_internal::kVerbose, T...> { 
  using log_internal::log<log_internal::kVerbose, T...>::log;

};



template <typename... T>

fatal(fmt::format_string<T...>, T&&...) -> fatal<T...>;

template <typename... T>

error(fmt::format_string<T...>, T&&...) -> error<T...>;

template <typename... T>
@@ -146,6 +149,77 @@ verbose(fmt::format_string<T...>, T&&...) -> verbose<T...>; 


#endif  // GCC / C++20



[[noreturn]] [[maybe_unused]] static void fatal(

    fmt::format_string<> fmt,

    log_internal::source_location location = log_internal::source_location()) {

  vlog(log_internal::kFatal, LOG_TAG, location,

       static_cast<fmt::string_view>(fmt), fmt::make_format_args());

  std::abort();  // Enforce [[noreturn]]

}



template <typename T0>

[[noreturn]] [[maybe_unused]] static void fatal(

    fmt::format_string<T0> fmt, T0&& arg0,

    log_internal::source_location location = log_internal::source_location()) {

  vlog(log_internal::kFatal, LOG_TAG, location,

       static_cast<fmt::string_view>(fmt),

       fmt::make_format_args(log_internal::format_replace(arg0)));

  std::abort();  // Enforce [[noreturn]]

}



template <typename T0, typename T1>

[[noreturn]] [[maybe_unused]] static void fatal(

    fmt::format_string<T0, T1> fmt, T0&& arg0, T1&& arg1,

    log_internal::source_location location = log_internal::source_location()) {

  vlog(log_internal::kFatal, LOG_TAG, location,

       static_cast<fmt::string_view>(fmt),

       fmt::make_format_args(log_internal::format_replace(arg0),

                             log_internal::format_replace(arg1)));

  std::abort();  // Enforce [[noreturn]]

}



template <typename T0, typename T1, typename T2>

[[noreturn]] [[maybe_unused]] static void fatal(

    fmt::format_string<T0, T1, T2> fmt, T0&& arg0, T1&& arg1, T2&& arg2,

    log_internal::source_location location = log_internal::source_location()) {

  vlog(log_internal::kFatal, LOG_TAG, location,

       static_cast<fmt::string_view>(fmt),

       fmt::make_format_args(log_internal::format_replace(arg0),

                             log_internal::format_replace(arg1),

                             log_internal::format_replace(arg2)));

  std::abort();  // Enforce [[noreturn]]

}



template <typename T0, typename T1, typename T2, typename T3>

[[noreturn]] [[maybe_unused]] static void fatal(

    fmt::format_string<T0, T1, T2, T3> fmt, T0&& arg0, T1&& arg1, T2&& arg2,

    T3&& arg3,

    log_internal::source_location location = log_internal::source_location()) {

  vlog(log_internal::kFatal, LOG_TAG, location,

       static_cast<fmt::string_view>(fmt),

       fmt::make_format_args(log_internal::format_replace(arg0),

                             log_internal::format_replace(arg1),

                             log_internal::format_replace(arg2),

                             log_internal::format_replace(arg3)));

  std::abort();  // Enforce [[noreturn]]

}



template <typename... T>

struct fatal_if {

  fatal_if(bool cond, fmt::format_string<T...> fmt, T&&... args,

           log_internal::source_location location =

               log_internal::source_location()) {

    if (cond) {

      vlog(log_internal::kFatal, LOG_TAG, location,

           static_cast<fmt::string_view>(fmt),

           fmt::make_format_args(format_replace(args)...));

    }

  }

};



template <typename... T>

fatal_if(fmt::format_string<T...>, T&&...) -> fatal_if<T...>;



}  // namespace bluetooth::log



namespace fmt {

system/log/src/vlog_android.cc

+5 −5
Original line number Diff line number Diff line
@@ -23,9 +23,8 @@ namespace bluetooth::log_internal { 


static constexpr size_t kBufferSize = 1024;



void vlog(Level level, char const* tag, char const* file_name, int line,

          char const* function_name, fmt::string_view fmt,

          fmt::format_args vargs) {

void vlog(Level level, char const* tag, source_location location,

          fmt::string_view fmt, fmt::format_args vargs) {

  // Check if log is enabled.

  if (!__android_log_is_loggable(level, tag, ANDROID_LOG_INFO) &&

      !__android_log_is_loggable(level, "bluetooth", ANDROID_LOG_INFO)) {
@@ -38,8 +37,9 @@ void vlog(Level level, char const* tag, char const* file_name, int line, 
  // In order to have consistent logs we include it manually in the log

  // message.

  truncating_buffer<kBufferSize> buffer;

  fmt::format_to(std::back_insert_iterator(buffer), "{}:{} {}: ", file_name,

                 line, function_name);

  fmt::format_to(std::back_insert_iterator(buffer),

                 "{}:{} {}: ", location.file_name, location.line,

                 location.function_name);

  fmt::vformat_to(std::back_insert_iterator(buffer), fmt, vargs);



  // Send message to liblog.

system/log/src/vlog_syslog.cc

+3 −4
Original line number Diff line number Diff line
@@ -48,9 +48,8 @@ Level GetDefaultLogLevel() { return gDefaultLogLevel; } 
// Default value for $MaxMessageSize for rsyslog.

static constexpr size_t kBufferSize = 8192;



void vlog(Level level, char const* tag, char const* file_name, int line,

          char const* function_name, fmt::string_view fmt,

          fmt::format_args vargs) {

void vlog(Level level, char const* tag, source_location location,

          fmt::string_view fmt, fmt::format_args vargs) {

  // Filter out logs that don't meet level requirement.

  Level current_level = GetLogLevelForTag(tag);

  if (level < current_level) {
@@ -84,7 +83,7 @@ void vlog(Level level, char const* tag, char const* file_name, int line, 


  // Format file, line.

  fmt::format_to(std::back_insert_iterator(buffer), "{} {}:{} {}: ", tag,

                 file_name, line, function_name);

                 location.file_name, location.line, location.function_name);



  // Format message.

  fmt::vformat_to(std::back_insert_iterator(buffer), fmt, vargs);

system/log/src/vlog_test.cc

+43 −3
Original line number Diff line number Diff line
@@ -118,13 +118,53 @@ TEST(BluetoothLoggerTest, error) { 
               "TestBody: error test");

}



TEST(BluetoothLoggerTest, fatal) {

  androidLogMessage.reset();



  ASSERT_DEATH(

      {

        log::fatal("fatal test");

        // Validate that the compiler is correctly handling log::fatal as

        // [[noreturn]] by attempting to invoke an undefined function.

        // This test will fail linking if this check fails.

        void undefined_function();

        undefined_function();

      },

      "fatal test");



  ASSERT_DEATH(

      {

        log::fatal("fatal test {}", "2");

        void undefined_function();

        undefined_function();

      },

      "fatal test 2");



  ASSERT_DEATH(

      {

        log::fatal("fatal test {}, {}", 2, 3);

        void undefined_function();

        undefined_function();

      },

      "fatal test 2, 3");

}



TEST(BluetoothLoggerTest, fatal_if) {

  androidLogMessage.reset();



  log::fatal_if(false, "fatal_if test false");



  ASSERT_DEATH(

      { log::fatal_if(true, "fatal_if test true"); }, "fatal_if test true");

}



TEST(BluetoothLoggerTest, null_string_parameter) {

  androidLogMessage.reset();



  char const* const_null_str = nullptr;

  log::info("input: {}", const_null_str);

  EXPECT_STREQ(androidLogMessage->message,

               "packages/modules/Bluetooth/system/log/src/vlog_test.cc:125 "

               "packages/modules/Bluetooth/system/log/src/vlog_test.cc:165 "

               "TestBody: input: (nullptr)");



  androidLogMessage.reset();
@@ -132,7 +172,7 @@ TEST(BluetoothLoggerTest, null_string_parameter) { 
  char* null_str = nullptr;

  log::info("input: {}", null_str);

  EXPECT_STREQ(androidLogMessage->message,

               "packages/modules/Bluetooth/system/log/src/vlog_test.cc:133 "

               "packages/modules/Bluetooth/system/log/src/vlog_test.cc:173 "

               "TestBody: input: (nullptr)");



  androidLogMessage.reset();
@@ -140,6 +180,6 @@ TEST(BluetoothLoggerTest, null_string_parameter) { 
  char const* nonnull_str = "hello world";

  log::info("input: {}", nonnull_str);

  EXPECT_STREQ(androidLogMessage->message,

               "packages/modules/Bluetooth/system/log/src/vlog_test.cc:141 "

               "packages/modules/Bluetooth/system/log/src/vlog_test.cc:181 "

               "TestBody: input: hello world");

}