Garbage chars in DLT logs

-std::string_view does not provide an accessor method that returns a null-terminated C-string. spdlog uses std::string_view so we need to use dlt_user_log_write_sized_utf8_string to write strings. DCBUG24-7229 Signed-off-by: Joel Winarske <[email protected]>
toyota-connected · Feb 9, 2024 · ee49745 · ee49745
1 parent 706a371
commit ee49745
Show file tree

Hide file tree

Showing 10 changed files with 81 additions and 96 deletions.
diff --git a/README.md b/README.md
@@ -45,7 +45,7 @@ Ubuntu packages
 
 Fedora packages
 
-    sudo dnf install dlt-libs-devel dlt-dameon dlt-tools
+    sudo dnf install dlt-libs-devel dlt-daemon dlt-tools
 
 ### Logging with DLT
 
@@ -62,7 +62,7 @@ Start new terminal
 
 You can enable the sanitizers with SANITIZE_ADDRESS, SANITIZE_MEMORY, SANITIZE_THREAD or SANITIZE_UNDEFINED options in your CMake configuration. You can do this by passing e.g. -DSANITIZE_ADDRESS=On on your command line.
 
-If sanitizers are supported by your compiler, the specified targets will be build with sanitizer support. If your compiler has no sanitizing capabilities you'll get a warning but CMake will continue processing and sanitizing will simply just be ignored.
+If sanitizers are supported by your compiler, the specified targets will be built with sanitizer support. If your compiler has no sanitizing capabilities you'll get a warning but CMake will continue processing and sanitizing will simply just be ignored.
 
 # Backend Support
 
@@ -82,7 +82,7 @@ Running Vulkan requires an engine version that supports Vulkan.  Stable does not
 
 # Bundle File Override Logic
 
-If an override file is not present, it gets loaded from default location.
+If an override file is not present, it gets loaded from a default location.
 
 ## Optional override files
 
@@ -114,9 +114,9 @@ Yocto/Desktop Default - https://tldp.org/HOWTO/Program-Library-HOWTO/shared-libr
 
 `--c` - Disables the cursor.
 
-`--d` - Outputs backend debug information.  If Vulkan and Validation Layer is available, it will be loaded.
+`--d` - Outputs backend debug information.  If Vulkan and Validation Layer are available, it will be loaded.
 
-`--f` - Sets window to fullscreen.
+`--f` - Sets the window to fullscreen.
 
 `--w={int value}` - Sets View width.  Requires an integer value.
 

diff --git a/shell/backend/egl.cc b/shell/backend/egl.cc
@@ -22,7 +22,6 @@
 
 #include <GLES2/gl2.h>
 
-#include "gl_process_resolver.h"
 #include "logging.h"
 
 Egl::Egl(void* native_display, int buffer_size, bool debug)
@@ -580,17 +579,9 @@ static struct egl_config_attribute egl_config_attributes[] = {
 };
 
 void Egl::ReportGlesAttributes(EGLConfig* configs, EGLint count) {
-  std::stringstream ss;
   spdlog::info("OpenGL ES Attributes:");
-  ss << "\tEGL_VENDOR: \"" << eglQueryString(m_dpy, EGL_VENDOR) << "\"";
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
-  ss << "\tEGL_CLIENT_APIS: \"" << eglQueryString(m_dpy, EGL_CLIENT_APIS)
-     << "\"";
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
+  spdlog::info("\tEGL_VENDOR: \"{}\"", eglQueryString(m_dpy, EGL_VENDOR));
+  spdlog::info("\tEGL_CLIENT_APIS: \"{}\"", eglQueryString(m_dpy, EGL_CLIENT_APIS));
   spdlog::info("\tEGL_EXTENSIONS:");
 
   print_extension_list(m_dpy);
@@ -602,6 +593,7 @@ void Egl::ReportGlesAttributes(EGLConfig* configs, EGLint count) {
     return;
   }
 
+  std::stringstream ss;
   spdlog::info("EGL framebuffer configurations:");
   for (EGLint i = 0; i < num_config; i++) {
     ss << "\tConfiguration #" << i;

diff --git a/shell/configuration/configuration.cc b/shell/configuration/configuration.cc
@@ -323,49 +323,25 @@ rapidjson::Document Configuration::getJsonDocument(
 }
 
 void Configuration::PrintConfig(const Config& config) {
-  std::stringstream ss;
-  ss << kGitBranch << " @ " << kGitCommitHash;
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
+  spdlog::info("{} @ {}", kGitBranch, kGitCommitHash);
 
   spdlog::info("**********");
   spdlog::info("* Global *");
   spdlog::info("**********");
   if (!config.app_id.empty()) {
-    ss << "Application Id: .......... " << config.app_id;
-    spdlog::info(ss.str().c_str());
-    ss.str("");
-    ss.clear();
+    spdlog::info("Application Id: .......... {}", config.app_id);
   }
   if (!config.json_configuration_path.empty()) {
-    ss << "JSON Configuration: ...... " << config.json_configuration_path;
-    spdlog::info(ss.str().c_str());
-    ss.str("");
-    ss.clear();
+    spdlog::info("JSON Configuration: ...... {}", config.json_configuration_path);
   }
   if (!config.cursor_theme.empty()) {
-    ss << "Cursor Theme: ............ " << config.cursor_theme;
-    spdlog::info(ss.str().c_str());
-    ss.str("");
-    ss.clear();
-  }
-  ss << "Disable Cursor: .......... "
-     << (config.disable_cursor ? "true" : "false");
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
+    spdlog::info("Cursor Theme: ............ {}", config.cursor_theme);
+  }
+  spdlog::info("Disable Cursor: .......... {}", (config.disable_cursor ? "true" : "false"));
   if (!config.wayland_event_mask.empty()) {
-    ss << "Wayland Event Mask: ...... " << config.wayland_event_mask;
-    spdlog::info(ss.str().c_str());
-    ss.str("");
-    ss.clear();
-  }
-  ss << "Debug Backend: ........... "
-     << (config.debug_backend ? "true" : "false");
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
+    spdlog::info("Wayland Event Mask: ...... {}", config.wayland_event_mask);
+  }
+  spdlog::info("Debug Backend: ........... {}", (config.debug_backend ? "true" : "false"));
   spdlog::info("********");
   spdlog::info("* View *");
   spdlog::info("********");
@@ -375,44 +351,18 @@ void Configuration::PrintConfig(const Config& config) {
       spdlog::info(arg);
     }
   }
-  ss << "Bundle Path: .............. " << config.view.bundle_path;
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
-  ss << "Window Type: .............. " << config.view.window_type;
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
-  ss << "Output Index: ............. " << config.view.wl_output_index;
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
-  ss << "Size: ..................... " << config.view.width << " x "
-     << config.view.height;
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
+  spdlog::info("Bundle Path: .............. {}", config.view.bundle_path);
+  spdlog::info("Window Type: .............. {}", config.view.window_type);
+  spdlog::info("Output Index: ............. {}", config.view.wl_output_index);
+  spdlog::info("Size: ..................... {} x {}", config.view.width, config.view.height);
   if (config.view.pixel_ratio != kDefaultPixelRatio) {
-    ss << "Pixel Ratio: .............. " << config.view.pixel_ratio;
-    spdlog::info(ss.str().c_str());
-    ss.str("");
-    ss.clear();
+    spdlog::info("Pixel Ratio: .............. {}", config.view.pixel_ratio);
   }
   if (config.view.ivi_surface_id > 0) {
-    ss << "IVI Surface ID: ........... " << config.view.ivi_surface_id;
-    spdlog::info(ss.str().c_str());
-    ss.str("");
-    ss.clear();
-  }
-  ss << "Fullscreen: ............... "
-     << (config.view.fullscreen ? "true" : "false");
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
-  ss << "Accessibility Features: ... " << config.view.accessibility_features;
-  spdlog::info(ss.str().c_str());
-  ss.str("");
-  ss.clear();
+    spdlog::info("IVI Surface ID: ........... {}", config.view.ivi_surface_id);
+  }
+  spdlog::info("Fullscreen: ............... {}", (config.view.fullscreen ? "true" : "false"));
+  spdlog::info("Accessibility Features: ... {}", config.view.accessibility_features);
 }
 
 Configuration::Config Configuration::ConfigFromArgcArgv(

diff --git a/shell/engine.cc b/shell/engine.cc
@@ -151,11 +151,7 @@ Engine::Engine(FlutterView* view,
       m_args.aot_data = m_aot_data;
     }
   } else {
-    std::stringstream ss_;
-    ss_ << "(" << m_index << ") Runtime=debug";
-    spdlog::info(ss_.str().c_str());
-    ss_.str("");
-    ss_.clear();
+    spdlog::info("({}) Runtime=debug", m_index);
     std::string kernel_snapshot =
         fml::paths::JoinPaths({m_assets_path, "kernel_blob.bin"});
     if (!fml::IsFile(kernel_snapshot)) {

diff --git a/shell/logging/dlt/dlt.cc b/shell/logging/dlt/dlt.cc
@@ -87,3 +87,25 @@ void Dlt::LogString(DltLogLevelType log_level, const char* buff) {
     std::cerr.flush();
   }
 }
+
+MAYBE_UNUSED
+void Dlt::LogSizedString(DltLogLevelType log_level, const char* buff, uint16_t length) {
+  if (gContextSet && length == 0) {
+    LogString(log_level, buff);
+  }
+  else if (gContextSet && length) {
+    DltContextData log_local;
+    auto res = LibDlt->UserLogWriteStart(&gContext, &log_local, log_level);
+    if (res == DltReturnValue::True) {
+      (void)LibDlt->UserLogWriteSizedUtf8String(&log_local, buff, length);
+      (void)LibDlt->UserLogWriteFinish(&log_local);
+    }
+
+    if (log_level == DltLogLevelType::LOG_FATAL) {
+      Dlt::Unregister();
+    }
+  } else {
+    std::cerr << buff;
+    std::cerr.flush();
+  }
+}
diff --git a/shell/logging/dlt/dlt.h b/shell/logging/dlt/dlt.h
@@ -61,6 +61,17 @@ class Dlt {
    */
   static void LogString(DltLogLevelType log_level, const char* buff);
 
+  /**
+   * @brief Convert a specified string to DLT log string specifying a buffer length.
+   * @param[in] log_level DLT log level.
+   * @param[in] buff String to be converted.
+   * @param[in] length length of buffer.
+   * @return void
+   * @relation
+   * dlt
+   */
+  static void LogSizedString(DltLogLevelType log_level, const char* buff, uint16_t length);
+
  private:
   FML_DISALLOW_COPY_AND_ASSIGN(Dlt);
 };
diff --git a/shell/logging/dlt/libdlt.cc b/shell/logging/dlt/libdlt.cc
@@ -35,6 +35,8 @@ LibDltExports::LibDltExports(void* lib) {
     GetFuncAddress(lib, "dlt_user_log_write_int16", &UserLogWriteInt16);
     GetFuncAddress(lib, "dlt_user_log_write_int32", &UserLogWriteInt32);
     GetFuncAddress(lib, "dlt_user_log_write_int64", &UserLogWriteInt64);
+    GetFuncAddress(lib, "dlt_user_log_write_constant_utf8_string", &UserLogWriteConstantUtf8String);
+    GetFuncAddress(lib, "dlt_user_log_write_sized_utf8_string", &UserLogWriteSizedUtf8String);
   }
 }
 

diff --git a/shell/logging/dlt/libdlt.h b/shell/logging/dlt/libdlt.h
@@ -87,6 +87,8 @@ struct LibDltExports {
                                       DltLogLevelType) = nullptr;
   DltReturnValue (*UserLogWriteFinish)(DltContextData*) = nullptr;
   DltReturnValue (*UserLogWriteString)(DltContextData*, const char*) = nullptr;
+  DltReturnValue (*UserLogWriteConstantUtf8String)(DltContextData *, const char *) = nullptr;
+  DltReturnValue (*UserLogWriteSizedUtf8String)(DltContextData *, const char *, uint16_t) = nullptr;
   DltReturnValue (*UserLogWriteInt)(DltContextData*, int) = nullptr;
   DltReturnValue (*UserLogWriteInt8)(DltContextData*, int8_t) = nullptr;
   DltReturnValue (*UserLogWriteInt16)(DltContextData*, int16_t) = nullptr;

diff --git a/shell/logging/logging.cc b/shell/logging/logging.cc
@@ -20,7 +20,6 @@
 #if defined(ENABLE_DLT)
 #include <spdlog/sinks/callback_sink.h>
 #endif
-#include <spdlog/sinks/ringbuffer_sink.h>
 
 #include <constants.h>
 
@@ -32,22 +31,32 @@ Logging::Logging() {
         "primary", [](const spdlog::details::log_msg& msg) {
           switch (msg.level) {
             case SPDLOG_LEVEL_TRACE:
-              Dlt::LogString(DltLogLevelType::LOG_VERBOSE, msg.payload.data());
+              Dlt::LogSizedString(DltLogLevelType::LOG_VERBOSE,
+                                  msg.payload.data(),
+                                  static_cast<uint16_t>(msg.payload.size()));
               break;
             case SPDLOG_LEVEL_DEBUG:
-              Dlt::LogString(DltLogLevelType::LOG_DEBUG, msg.payload.data());
+              Dlt::LogSizedString(DltLogLevelType::LOG_DEBUG,
+                                  msg.payload.data(),
+                                  static_cast<uint16_t>(msg.payload.size()));
               break;
             case SPDLOG_LEVEL_INFO:
-              Dlt::LogString(DltLogLevelType::LOG_INFO, msg.payload.data());
+              Dlt::LogSizedString(DltLogLevelType::LOG_INFO, msg.payload.data(),
+                                  static_cast<uint16_t>(msg.payload.size()));
               break;
             case SPDLOG_LEVEL_WARN:
-              Dlt::LogString(DltLogLevelType::LOG_WARN, msg.payload.data());
+              Dlt::LogSizedString(DltLogLevelType::LOG_WARN, msg.payload.data(),
+                                  static_cast<uint16_t>(msg.payload.size()));
               break;
             case SPDLOG_LEVEL_ERROR:
-              Dlt::LogString(DltLogLevelType::LOG_ERROR, msg.payload.data());
+              Dlt::LogSizedString(DltLogLevelType::LOG_ERROR,
+                                  msg.payload.data(),
+                                  static_cast<uint16_t>(msg.payload.size()));
               break;
             case SPDLOG_LEVEL_CRITICAL:
-              Dlt::LogString(DltLogLevelType::LOG_FATAL, msg.payload.data());
+              Dlt::LogSizedString(DltLogLevelType::LOG_FATAL,
+                                  msg.payload.data(),
+                                  static_cast<uint16_t>(msg.payload.size()));
               break;
             default:
               break;

diff --git a/shell/plugins/keyboard_manager/keyboard_manager.cc b/shell/plugins/keyboard_manager/keyboard_manager.cc
@@ -18,6 +18,7 @@
 
 #include "engine.h"
 #include "keyboard_manager.h"
+#include "standard_method_codec.h"
 
 void KeyboardManager::OnPlatformMessage(const FlutterPlatformMessage* message,
                                  void* userdata) {