Merge "liblog: correct new APIs per the API review" into rvc-dev

base/liblog_symbols.cpp

@@ -48,7 +48,7 @@ const std::optional<LibLogFunctions>& GetLibLogFunctions() {
   }
 
     DLSYM(__android_log_set_logger)
-    DLSYM(__android_log_write_logger_data)
+    DLSYM(__android_log_write_log_message)
     DLSYM(__android_log_logd_logger)
     DLSYM(__android_log_stderr_logger)
     DLSYM(__android_log_set_aborter)
@@ -71,7 +71,7 @@ const std::optional<LibLogFunctions>& GetLibLogFunctions() {
   static std::optional<LibLogFunctions> liblog_functions = []() -> std::optional<LibLogFunctions> {
     return LibLogFunctions{
         .__android_log_set_logger = __android_log_set_logger,
-        .__android_log_write_logger_data = __android_log_write_logger_data,
+        .__android_log_write_log_message = __android_log_write_log_message,
         .__android_log_logd_logger = __android_log_logd_logger,
         .__android_log_stderr_logger = __android_log_stderr_logger,
         .__android_log_set_aborter = __android_log_set_aborter,

base/liblog_symbols.h

@@ -25,13 +25,10 @@ namespace base {
 
 struct LibLogFunctions {
   void (*__android_log_set_logger)(__android_logger_function logger);
-  void (*__android_log_write_logger_data)(struct __android_logger_data* logger_data,
+  void (*__android_log_write_log_message)(struct __android_log_message* log_message);
-                                          const char* msg);
 
-  void (*__android_log_logd_logger)(const struct __android_logger_data* logger_data,
+  void (*__android_log_logd_logger)(const struct __android_log_message* log_message);
-                                    const char* msg);
+  void (*__android_log_stderr_logger)(const struct __android_log_message* log_message);
-  void (*__android_log_stderr_logger)(const struct __android_logger_data* logger_data,
-                                      const char* message);
 
   void (*__android_log_set_aborter)(__android_aborter_function aborter);
   void (*__android_log_call_aborter)(const char* abort_message);

base/logging.cpp

@@ -349,9 +349,9 @@ void LogdLogger::operator()(LogId id, LogSeverity severity, const char* tag,
 
   static auto& liblog_functions = GetLibLogFunctions();
   if (liblog_functions) {
-    __android_logger_data logger_data = {sizeof(__android_logger_data),     lg_id, priority, tag,
+    __android_log_message log_message = {sizeof(__android_log_message),     lg_id, priority, tag,
-                                         static_cast<const char*>(nullptr), 0};
+                                         static_cast<const char*>(nullptr), 0,     message};
-    liblog_functions->__android_log_logd_logger(&logger_data, message);
+    liblog_functions->__android_log_logd_logger(&log_message);
   } else {
     __android_log_buf_print(lg_id, priority, tag, "%s", message);
   }
@@ -426,13 +426,13 @@ void SetLogger(LogFunction&& logger) {
     // std::function<>, which is the not-thread-safe alternative.
     static std::atomic<LogFunction*> logger_function(nullptr);
     auto* old_logger_function = logger_function.exchange(new LogFunction(logger));
-    liblog_functions->__android_log_set_logger([](const struct __android_logger_data* logger_data,
+    liblog_functions->__android_log_set_logger([](const struct __android_log_message* log_message) {
-                                                  const char* message) {
+      auto log_id = log_id_tToLogId(log_message->buffer_id);
-      auto log_id = log_id_tToLogId(logger_data->buffer_id);
+      auto severity = PriorityToLogSeverity(log_message->priority);
-      auto severity = PriorityToLogSeverity(logger_data->priority);
 
       auto& function = *logger_function.load(std::memory_order_acquire);
-      function(log_id, severity, logger_data->tag, logger_data->file, logger_data->line, message);
+      function(log_id, severity, log_message->tag, log_message->file, log_message->line,
+               log_message->message);
     });
     delete old_logger_function;
   } else {
@@ -576,9 +576,9 @@ void LogMessage::LogLine(const char* file, unsigned int line, LogSeverity severi
   static auto& liblog_functions = GetLibLogFunctions();
   int32_t priority = LogSeverityToPriority(severity);
   if (liblog_functions) {
-    __android_logger_data logger_data = {
+    __android_log_message log_message = {
-        sizeof(__android_logger_data), LOG_ID_DEFAULT, priority, tag, file, line};
+        sizeof(__android_log_message), LOG_ID_DEFAULT, priority, tag, file, line, message};
-    liblog_functions->__android_log_write_logger_data(&logger_data, message);
+    liblog_functions->__android_log_write_log_message(&log_message);
   } else {
     if (tag == nullptr) {
       std::lock_guard<std::recursive_mutex> lock(TagLock());

liblog/include/android/log.h

@@ -184,20 +184,21 @@ int __android_log_buf_print(int bufID, int prio, const char* tag, const char* fm
  * Logger data struct used for writing log messages to liblog via __android_log_write_logger_data()
  * and sending log messages to user defined loggers specified in __android_log_set_logger().
  */
-struct __android_logger_data {
+struct __android_log_message {
-  size_t struct_size; /* Must be set to sizeof(__android_logger_data) and is used for versioning. */
+  size_t
-  int32_t buffer_id;  /* log_id_t or -1 to represent 'default'. */
+      struct_size;   /** Must be set to sizeof(__android_log_message) and is used for versioning. */
-  int32_t priority;   /* android_LogPriority values. */
+  int32_t buffer_id; /** {@link log_id_t} values. */
-  const char* tag;
+  int32_t priority;  /** {@link android_LogPriority} values. */
-  const char* file;  /* Optional file name, may be set to nullptr. */
+  const char* tag;   /** The tag for the log message. */
-  uint32_t line;     /* Optional line number, ignore if file is nullptr. */
+  const char* file;  /** Optional file name, may be set to nullptr. */
+  uint32_t line;     /** Optional line number, ignore if file is nullptr. */
+  const char* message; /** The log message itself. */
 };
 
 /**
  * Prototype for the 'logger' function that is called for every log message.
  */
-typedef void (*__android_logger_function)(const struct __android_logger_data* logger_data,
+typedef void (*__android_logger_function)(const struct __android_log_message* log_message);
-                                          const char* message);
 /**
  * Prototype for the 'abort' function that is called when liblog will abort due to
  * __android_log_assert() failures.
@@ -206,52 +207,85 @@ typedef void (*__android_aborter_function)(const char* abort_message);
 
 #if !defined(__ANDROID__) || __ANDROID_API__ >= 30
 /**
- * Writes the log message specified with logger_data and msg to the log.  logger_data includes
+ * Writes the log message specified by log_message.  log_message includes additional file name and
- * additional file name and line number information that a logger may use.  logger_data is versioned
+ * line number information that a logger may use.  log_message is versioned for backwards
- * for backwards compatibility.
+ * compatibility.
  * This assumes that loggability has already been checked through __android_log_is_loggable().
  * Higher level logging libraries, such as libbase, first check loggability, then format their
  * buffers, then pass the message to liblog via this function, and therefore we do not want to
  * duplicate the loggability check here.
+ *
+ * @param log_message the log message itself, see {@link __android_log_message}.
+ *
+ * Available since API level 30.
  */
-void __android_log_write_logger_data(struct __android_logger_data* logger_data, const char* msg)
+void __android_log_write_log_message(struct __android_log_message* log_message) __INTRODUCED_IN(30);
-    __INTRODUCED_IN(30);
 
 /**
  * Sets a user defined logger function.  All log messages sent to liblog will be set to the
- * function pointer specified by logger for processing.
+ * function pointer specified by logger for processing.  It is not expected that log messages are
+ * already terminated with a new line.  This function should add new lines if required for line
+ * separation.
+ *
+ * @param logger the new function that will handle log messages.
+ *
+ * Available since API level 30.
  */
 void __android_log_set_logger(__android_logger_function logger) __INTRODUCED_IN(30);
 
 /**
  * Writes the log message to logd.  This is an __android_logger_function and can be provided to
  * __android_log_set_logger().  It is the default logger when running liblog on a device.
+ *
+ * @param log_message the log message to write, see {@link __android_log_message}.
+ *
+ * Available since API level 30.
  */
-void __android_log_logd_logger(const struct __android_logger_data* logger_data, const char* msg)
+void __android_log_logd_logger(const struct __android_log_message* log_message) __INTRODUCED_IN(30);
-    __INTRODUCED_IN(30);
 
 /**
  * Writes the log message to stderr.  This is an __android_logger_function and can be provided to
  * __android_log_set_logger().  It is the default logger when running liblog on host.
+ *
+ * @param log_message the log message to write, see {@link __android_log_message}.
+ *
+ * Available since API level 30.
  */
-void __android_log_stderr_logger(const struct __android_logger_data* logger_data,
+void __android_log_stderr_logger(const struct __android_log_message* log_message)
-                                 const char* message) __INTRODUCED_IN(30);
+    __INTRODUCED_IN(30);
 
 /**
- * Sets a user defined aborter function that is called for __android_log_assert() failures.
+ * Sets a user defined aborter function that is called for __android_log_assert() failures.  This
+ * user defined aborter function is highly recommended to abort and be noreturn, but is not strictly
+ * required to.
+ *
+ * @param aborter the new aborter function, see {@link __android_aborter_function}.
+ *
+ * Available since API level 30.
  */
 void __android_log_set_aborter(__android_aborter_function aborter) __INTRODUCED_IN(30);
 
 /**
  * Calls the stored aborter function.  This allows for other logging libraries to use the same
  * aborter function by calling this function in liblog.
+ *
+ * @param abort_message an additional message supplied when aborting, for example this is used to
+ *                      call android_set_abort_message() in __android_log_default_aborter().
+ *
+ * Available since API level 30.
  */
 void __android_log_call_aborter(const char* abort_message) __INTRODUCED_IN(30);
 
 /**
  * Sets android_set_abort_message() on device then aborts().  This is the default aborter.
+ *
+ * @param abort_message an additional message supplied when aborting.  This functions calls
+ *                      android_set_abort_message() with its contents.
+ *
+ * Available since API level 30.
  */
-void __android_log_default_aborter(const char* abort_message) __INTRODUCED_IN(30);
+void __android_log_default_aborter(const char* abort_message) __attribute__((noreturn))
+__INTRODUCED_IN(30);
 
 /**
  * Use the per-tag properties "log.tag.<tagname>" along with the minimum priority from
@@ -263,7 +297,13 @@ void __android_log_default_aborter(const char* abort_message) __INTRODUCED_IN(30
  * minimum priority needed to log.  If only one is set, then that value is used to determine the
  * minimum priority needed.  If none are set, then default_priority is used.
  *
- * prio is ANDROID_LOG_VERBOSE to ANDROID_LOG_FATAL.
+ * @param prio         the priority to test, takes {@link android_LogPriority} values.
+ * @param tag          the tag to test.
+ * @param len          the length of the tag.
+ * @param default_prio the default priority to use if no properties or minimum priority are set.
+ * @return an integer where 1 indicates that the message is loggable and 0 indicates that it is not.
+ *
+ * Available since API level 30.
  */
 int __android_log_is_loggable(int prio, const char* tag, int default_prio) __INTRODUCED_IN(30);
 int __android_log_is_loggable_len(int prio, const char* tag, size_t len, int default_prio)
@@ -272,13 +312,22 @@ int __android_log_is_loggable_len(int prio, const char* tag, size_t len, int def
 /**
  * Sets the minimum priority that will be logged for this process.
  *
- * This returns the previous set minimum priority, or ANDROID_LOG_DEFAULT if none was set.
+ * @param priority the new minimum priority to set, takes @{link android_LogPriority} values.
+ * @return the previous set minimum priority as @{link android_LogPriority} values, or
+ *         ANDROID_LOG_DEFAULT if none was set.
+ *
+ * Available since API level 30.
  */
 int32_t __android_log_set_minimum_priority(int32_t priority) __INTRODUCED_IN(30);
 
 /**
  * Gets the minimum priority that will be logged for this process.  If none has been set by a
  * previous __android_log_set_minimum_priority() call, this returns ANDROID_LOG_DEFAULT.
+ *
+ * @return the current minimum priority as @{link android_LogPriority} values, or
+ *         ANDROID_LOG_DEFAULT if none is set.
+ *
+ * Available since API level 30.
  */
 int32_t __android_log_get_minimum_priority(void) __INTRODUCED_IN(30);
 
@@ -286,6 +335,10 @@ int32_t __android_log_get_minimum_priority(void) __INTRODUCED_IN(30);
  * Sets the default tag if no tag is provided when writing a log message.  Defaults to
  * getprogname().  This truncates tag to the maximum log message size, though appropriate tags
  * should be much smaller.
+ *
+ * @param tag the new log tag.
+ *
+ * Available since API level 30.
  */
 void __android_log_set_default_tag(const char* tag) __INTRODUCED_IN(30);
 #endif

liblog/liblog.map.txt

@@ -77,7 +77,7 @@ LIBLOG_R { # introduced=30
     __android_log_set_logger;
     __android_log_set_minimum_priority;
     __android_log_stderr_logger;
-    __android_log_write_logger_data;
+    __android_log_write_log_message;
 };
 
 LIBLOG_PRIVATE {

liblog/logger_write.cpp

@@ -250,8 +250,7 @@ static uint64_t GetThreadId() {
 #endif
 }
 
-void __android_log_stderr_logger(const struct __android_logger_data* logger_data,
+void __android_log_stderr_logger(const struct __android_log_message* log_message) {
-                                 const char* message) {
   struct tm now;
   time_t t = time(nullptr);
 
@@ -268,33 +267,32 @@ void __android_log_stderr_logger(const struct __android_logger_data* logger_data
   static_assert(arraysize(log_characters) - 1 == ANDROID_LOG_SILENT,
                 "Mismatch in size of log_characters and values in android_LogPriority");
   int32_t priority =
-      logger_data->priority > ANDROID_LOG_SILENT ? ANDROID_LOG_FATAL : logger_data->priority;
+      log_message->priority > ANDROID_LOG_SILENT ? ANDROID_LOG_FATAL : log_message->priority;
   char priority_char = log_characters[priority];
   uint64_t tid = GetThreadId();
 
-  if (logger_data->file != nullptr) {
+  if (log_message->file != nullptr) {
     fprintf(stderr, "%s %c %s %5d %5" PRIu64 " %s:%u] %s\n",
-            logger_data->tag ? logger_data->tag : "nullptr", priority_char, timestamp, getpid(),
+            log_message->tag ? log_message->tag : "nullptr", priority_char, timestamp, getpid(),
-            tid, logger_data->file, logger_data->line, message);
+            tid, log_message->file, log_message->line, log_message->message);
   } else {
     fprintf(stderr, "%s %c %s %5d %5" PRIu64 " %s\n",
-            logger_data->tag ? logger_data->tag : "nullptr", priority_char, timestamp, getpid(),
+            log_message->tag ? log_message->tag : "nullptr", priority_char, timestamp, getpid(),
-            tid, message);
+            tid, log_message->message);
   }
 }
 
-void __android_log_logd_logger(const struct __android_logger_data* logger_data,
+void __android_log_logd_logger(const struct __android_log_message* log_message) {
-                               const char* message) {
+  int buffer_id = log_message->buffer_id == LOG_ID_DEFAULT ? LOG_ID_MAIN : log_message->buffer_id;
-  int buffer_id = logger_data->buffer_id == LOG_ID_DEFAULT ? LOG_ID_MAIN : logger_data->buffer_id;
 
   struct iovec vec[3];
   vec[0].iov_base =
-      const_cast<unsigned char*>(reinterpret_cast<const unsigned char*>(&logger_data->priority));
+      const_cast<unsigned char*>(reinterpret_cast<const unsigned char*>(&log_message->priority));
   vec[0].iov_len = 1;
-  vec[1].iov_base = const_cast<void*>(static_cast<const void*>(logger_data->tag));
+  vec[1].iov_base = const_cast<void*>(static_cast<const void*>(log_message->tag));
-  vec[1].iov_len = strlen(logger_data->tag) + 1;
+  vec[1].iov_len = strlen(log_message->tag) + 1;
-  vec[2].iov_base = const_cast<void*>(static_cast<const void*>(message));
+  vec[2].iov_base = const_cast<void*>(static_cast<const void*>(log_message->message));
-  vec[2].iov_len = strlen(message) + 1;
+  vec[2].iov_len = strlen(log_message->message) + 1;
 
   write_to_log(static_cast<log_id_t>(buffer_id), vec, 3);
 }
@@ -303,29 +301,29 @@ int __android_log_write(int prio, const char* tag, const char* msg) {
   return __android_log_buf_write(LOG_ID_MAIN, prio, tag, msg);
 }
 
-void __android_log_write_logger_data(__android_logger_data* logger_data, const char* msg) {
+void __android_log_write_log_message(__android_log_message* log_message) {
   ErrnoRestorer errno_restorer;
 
-  if (logger_data->buffer_id != LOG_ID_DEFAULT && logger_data->buffer_id != LOG_ID_MAIN &&
+  if (log_message->buffer_id != LOG_ID_DEFAULT && log_message->buffer_id != LOG_ID_MAIN &&
-      logger_data->buffer_id != LOG_ID_SYSTEM && logger_data->buffer_id != LOG_ID_RADIO &&
+      log_message->buffer_id != LOG_ID_SYSTEM && log_message->buffer_id != LOG_ID_RADIO &&
-      logger_data->buffer_id != LOG_ID_CRASH) {
+      log_message->buffer_id != LOG_ID_CRASH) {
     return;
   }
 
   auto tag_lock = std::shared_lock{default_tag_lock, std::defer_lock};
-  if (logger_data->tag == nullptr) {
+  if (log_message->tag == nullptr) {
     tag_lock.lock();
-    logger_data->tag = GetDefaultTag().c_str();
+    log_message->tag = GetDefaultTag().c_str();
   }
 
 #if __BIONIC__
-  if (logger_data->priority == ANDROID_LOG_FATAL) {
+  if (log_message->priority == ANDROID_LOG_FATAL) {
-    android_set_abort_message(msg);
+    android_set_abort_message(log_message->message);
   }
 #endif
 
   auto lock = std::shared_lock{logger_function_lock};
-  logger_function(logger_data, msg);
+  logger_function(log_message);
 }
 
 int __android_log_buf_write(int bufID, int prio, const char* tag, const char* msg) {
@@ -335,8 +333,9 @@ int __android_log_buf_write(int bufID, int prio, const char* tag, const char* ms
     return 0;
   }
 
-  __android_logger_data logger_data = {sizeof(__android_logger_data), bufID, prio, tag, nullptr, 0};
+  __android_log_message log_message = {
-  __android_log_write_logger_data(&logger_data, msg);
+      sizeof(__android_log_message), bufID, prio, tag, nullptr, 0, msg};
+  __android_log_write_log_message(&log_message);
   return 1;
 }
 
@@ -351,9 +350,9 @@ int __android_log_vprint(int prio, const char* tag, const char* fmt, va_list ap)
 
   vsnprintf(buf, LOG_BUF_SIZE, fmt, ap);
 
-  __android_logger_data logger_data = {
+  __android_log_message log_message = {
-      sizeof(__android_logger_data), LOG_ID_MAIN, prio, tag, nullptr, 0};
+      sizeof(__android_log_message), LOG_ID_MAIN, prio, tag, nullptr, 0, buf};
-  __android_log_write_logger_data(&logger_data, buf);
+  __android_log_write_log_message(&log_message);
   return 1;
 }
 
@@ -371,9 +370,9 @@ int __android_log_print(int prio, const char* tag, const char* fmt, ...) {
   vsnprintf(buf, LOG_BUF_SIZE, fmt, ap);
   va_end(ap);
 
-  __android_logger_data logger_data = {
+  __android_log_message log_message = {
-      sizeof(__android_logger_data), LOG_ID_MAIN, prio, tag, nullptr, 0};
+      sizeof(__android_log_message), LOG_ID_MAIN, prio, tag, nullptr, 0, buf};
-  __android_log_write_logger_data(&logger_data, buf);
+  __android_log_write_log_message(&log_message);
   return 1;
 }
 
@@ -391,8 +390,9 @@ int __android_log_buf_print(int bufID, int prio, const char* tag, const char* fm
   vsnprintf(buf, LOG_BUF_SIZE, fmt, ap);
   va_end(ap);
 
-  __android_logger_data logger_data = {sizeof(__android_logger_data), bufID, prio, tag, nullptr, 0};
+  __android_log_message log_message = {
-  __android_log_write_logger_data(&logger_data, buf);
+      sizeof(__android_log_message), bufID, prio, tag, nullptr, 0, buf};
+  __android_log_write_log_message(&log_message);
   return 1;
 }
 

liblog/tests/liblog_global_state.cpp

@@ -59,16 +59,15 @@ TEST(liblog_global_state, libbase_logs_with_liblog_set_logger) {
   static unsigned int expected_line;
   static std::string expected_message = "libbase test message";
 
-  auto liblog_logger_function = [](const struct __android_logger_data* logger_data,
+  auto liblog_logger_function = [](const struct __android_log_message* log_message) {
-                                   const char* message) {
     message_seen = true;
-    EXPECT_EQ(sizeof(__android_logger_data), logger_data->struct_size);
+    EXPECT_EQ(sizeof(__android_log_message), log_message->struct_size);
-    EXPECT_EQ(LOG_ID_DEFAULT, logger_data->buffer_id);
+    EXPECT_EQ(LOG_ID_DEFAULT, log_message->buffer_id);
-    EXPECT_EQ(ANDROID_LOG_WARN, logger_data->priority);
+    EXPECT_EQ(ANDROID_LOG_WARN, log_message->priority);
-    EXPECT_STREQ(LOG_TAG, logger_data->tag);
+    EXPECT_STREQ(LOG_TAG, log_message->tag);
-    EXPECT_EQ(expected_file, logger_data->file);
+    EXPECT_EQ(expected_file, log_message->file);
-    EXPECT_EQ(expected_line, logger_data->line);
+    EXPECT_EQ(expected_line, log_message->line);
-    EXPECT_EQ(expected_message, message);
+    EXPECT_EQ(expected_message, log_message->message);
   };
 
   __android_log_set_logger(liblog_logger_function);
@@ -111,16 +110,15 @@ TEST(liblog_global_state, liblog_logs_with_liblog_set_logger) {
   static int expected_priority = ANDROID_LOG_WARN;
   static std::string expected_message = "libbase test message";
 
-  auto liblog_logger_function = [](const struct __android_logger_data* logger_data,
+  auto liblog_logger_function = [](const struct __android_log_message* log_message) {
-                                   const char* message) {
     message_seen = true;
-    EXPECT_EQ(sizeof(__android_logger_data), logger_data->struct_size);
+    EXPECT_EQ(sizeof(__android_log_message), log_message->struct_size);
-    EXPECT_EQ(expected_buffer_id, logger_data->buffer_id);
+    EXPECT_EQ(expected_buffer_id, log_message->buffer_id);
-    EXPECT_EQ(expected_priority, logger_data->priority);
+    EXPECT_EQ(expected_priority, log_message->priority);
-    EXPECT_STREQ(LOG_TAG, logger_data->tag);
+    EXPECT_STREQ(LOG_TAG, log_message->tag);
-    EXPECT_STREQ(nullptr, logger_data->file);
+    EXPECT_STREQ(nullptr, log_message->file);
-    EXPECT_EQ(0U, logger_data->line);
+    EXPECT_EQ(0U, log_message->line);
-    EXPECT_EQ(expected_message, message);
+    EXPECT_EQ(expected_message, log_message->message);
   };
 
   __android_log_set_logger(liblog_logger_function);