task-4660 std::format instead of snprintf(buffer...) in 9 files

examples/mir_demo_server/server_example.cpp

@@ -47,6 +47,7 @@
 
 #include <chrono>
 #include <cstdlib>
+#include <format>
 #include <linux/input-event-codes.h>
 
 #include <miral/sticky_keys.h>
@@ -105,14 +106,13 @@ catch (mir::AbnormalExit const& /*error*/)
 }
 catch (std::exception const& error)
 {
-    char const command_fmt[] = "/usr/share/apport/recoverable_problem --pid %d";
-    char command[sizeof(command_fmt)+32];
-    snprintf(command, sizeof(command), command_fmt, getpid());
+    auto const command = std::format(
+        "/usr/share/apport/recoverable_problem --pid {}", getpid());
     char const options[] = "we";
     char const key[] = "UnhandledException";
     auto const value = boost::diagnostic_information(error);
 
-    if (auto const output = popen(command, options))
+    if (auto const output = popen(command.c_str(), options))
     {
         fwrite(key, sizeof(key), 1, output);            // the null terminator is used intentionally as a separator
         fwrite(value.c_str(), value.size(), 1, output);

examples/mir_demo_server/server_example_test_client.cpp

@@ -24,6 +24,7 @@
 #include <mir/options/option.h>
 
 #include <chrono>
+#include <format>
 #include <future>
 
 #include <sys/wait.h>
@@ -78,20 +79,17 @@ void check_exit_status(pid_t pid, mir::Server& server, Self* self, int& countdow
         }
         else
         {
-            char const format[] = "Client has exited with status %d";
-            char buffer[sizeof format + 10];
-            snprintf(buffer, sizeof buffer, format, exit_status);
-            ml::log(ml::Severity::informational, buffer, component);
+            auto const msg =
+                std::format("Client has exited with status {}", exit_status);
+            ml::log(ml::Severity::informational, msg, component);
             self->test_failed = true;
         }
     }
     else if (WIFSIGNALED(status))
     {
         auto const signal = WTERMSIG(status);
-        char const format[] = "Client terminated by signal %d";
-        char buffer[sizeof format];
-        snprintf(buffer, sizeof buffer, format, signal);
-        ml::log(ml::Severity::informational, buffer, component);
+        auto const msg = std::format("Client terminated by signal {}", signal);
+        ml::log(ml::Severity::informational, msg, component);
         self->test_failed = true;
     }
     else

src/common/logging/input_timestamp.cpp

@@ -15,8 +15,8 @@
  */
 
 #include <mir/logging/input_timestamp.h>
-#include <cstdio>
 #include <cstdlib>
+#include <format>
 
 using namespace std::chrono;
 
@@ -33,9 +33,8 @@ std::string mir::logging::input_timestamp(mir::time::Clock const& clock, std::ch
 
     char const* sign_suffix = (age_ns < 0) ? "in the future" : "ago";
 
-    char str[64];
-    snprintf(str, sizeof str, "%lld (%lld.%06lldms %s)",
-             when_ns, milliseconds, sub_milliseconds, sign_suffix);
+    auto const fmt_now = std::format("{} ({}.{:06}ms {})",
+        when_ns, milliseconds, sub_milliseconds, sign_suffix);
 
-    return std::string(str);
+    return fmt_now;
 }

src/core/logging/logger.cpp

@@ -18,6 +18,8 @@
 #include <mir/logging/logger.h>
 
 #include <iostream>
+#include <chrono>
+#include <format>
 #include <mutex>
 #include <cstdarg>
 #include <cstdio>
@@ -81,27 +83,45 @@ void ml::format_message(std::ostream& out, Severity severity, std::string const&
         "< - debug - > "
     };
 
-    timespec ts{};
-    clock_gettime(CLOCK_REALTIME, &ts);
-    char now[32];
-    auto offset = strftime(now, sizeof(now), "%F %T", localtime(&ts.tv_sec));
-    snprintf(now+offset, sizeof(now)-offset, ".%06ld", ts.tv_nsec / 1000);
-
+    std::string fmt_now;
+    auto now = std::chrono::system_clock::now();
+    auto now_us =
+        std::chrono::time_point_cast<std::chrono::microseconds>(now);
+    try
+    {
+        auto local =
+            std::chrono::zoned_time{std::chrono::current_zone(), now_us};
+        fmt_now = std::format("{:%F %T}", local);
+    }
+    catch (...)
+    {
+        auto now_time_t = std::chrono::system_clock::to_time_t(now);
+        auto micros = now_us.time_since_epoch().count() % 1000000;
+        char buf[32]{};
+        std::tm tm{};
+        size_t offset;
+        if (gmtime_r(&now_time_t, &tm) != nullptr &&
+            (offset = std::strftime(
+                buf, sizeof(buf), "%Y-%m-%d %H:%M:%S", &tm)))
+        {
+            auto eos = std::format_to_n(buf + offset, sizeof(buf) - offset,
+                ".{:06}", micros).out;
+            *eos = '\0';
+            fmt_now = buf;
+        }
+        else
+        {
+            fmt_now = "unknown-time";
+        }
+    }
     if (!out || !out.good())
     {
         std::cerr << "Failed to write to log file: " << errno << std::endl;
         return;
     }
 
-    out << "["
-        << now
-        << "] "
-        << lut[static_cast<int>(severity)]
-        << component
-        << ": "
-        << message
-        << std::endl;
-
+    out << std::format("[{}] {}{}: {}\n",
+        fmt_now, lut[static_cast<int>(severity)], component, message);
 }
 
 namespace mir

src/server/frontend_xwayland/xwayland_spawner.cpp

@@ -26,6 +26,9 @@
 #include <mir/fatal.h>
 #include <mir/thread_name.h>
 
+#include <array>
+#include <format>
+#include <string_view>
 #include <fcntl.h>
 #include <sys/socket.h>
 #include <sys/un.h>
@@ -36,17 +39,14 @@ namespace md = mir::dispatch;
 
 namespace
 {
-auto const x11_lock_fmt = "/tmp/.X%d-lock";
-auto const x11_socket_fmt = "/tmp/.X11-unix/X%d";
+constexpr std::string_view x11_lock_fmt = "/tmp/.X{}-lock";
+constexpr std::string_view x11_socket_fmt = "/tmp/.X11-unix/X{}";
 
-// TODO this can be written with more modern c++
 int create_lockfile(int xdisplay)
 {
-    char lockfile[256];
+    auto const lockfile = std::format(x11_lock_fmt, xdisplay);
 
-    snprintf(lockfile, sizeof lockfile, x11_lock_fmt, xdisplay);
-
-    mir::Fd const fd{open(lockfile, O_WRONLY | O_CLOEXEC | O_CREAT | O_EXCL, 0444)};
+    mir::Fd const fd{open(lockfile.c_str(), O_WRONLY | O_CLOEXEC | O_CREAT | O_EXCL, 0444)};
     if (fd < 0)
         return EEXIST;
 
@@ -58,12 +58,11 @@ int create_lockfile(int xdisplay)
     bool const success_writing_to_lock_file = dprintf(fd, "%10lu\n", (unsigned long) getpid()) == 11;
 
     // Check if anyone else has created the socket (even though we have the lockfile)
-    char x11_socket[256];
-    snprintf(x11_socket, sizeof x11_socket, x11_socket_fmt, xdisplay);
+    auto const x11_socket = std::format(x11_socket_fmt, xdisplay);
 
-    if (!success_writing_to_lock_file || access(x11_socket, F_OK) == 0)
+    if (!success_writing_to_lock_file || access(x11_socket.c_str(), F_OK) == 0)
     {
-        unlink(lockfile);
+        unlink(lockfile.c_str());
         return EEXIST;
     }
 
@@ -141,10 +140,16 @@ auto create_sockets(int xdisplay) -> std::vector<mir::Fd>
     struct sockaddr_un addr{ .sun_family = AF_UNIX, .sun_path = { 0 } };
     size_t path_size;
 
-    path_size = snprintf(addr.sun_path + 1, sizeof(addr.sun_path) - 1, x11_socket_fmt, xdisplay);
+    auto eos = std::format_to_n(addr.sun_path + 1, sizeof(addr.sun_path) - 2,
+        x11_socket_fmt, xdisplay).out;
+    *eos = '\0';
+    path_size = eos - (addr.sun_path + 1);
     create_socket(result, &addr, path_size);
 
-    path_size = snprintf(addr.sun_path, sizeof(addr.sun_path), x11_socket_fmt, xdisplay);
+    eos = std::format_to_n(addr.sun_path, sizeof(addr.sun_path) - 1,
+        x11_socket_fmt, xdisplay).out;
+    *eos = '\0';
+    path_size = eos - addr.sun_path;
     create_socket(result, &addr, path_size);
 
     return result;
@@ -194,11 +199,15 @@ mf::XWaylandSpawner::XWaylandSpawner(std::function<void()> spawn)
 
 mf::XWaylandSpawner::~XWaylandSpawner()
 {
-    char path[256];
-    snprintf(path, sizeof path, x11_lock_fmt, xdisplay);
-    unlink(path);
-    snprintf(path, sizeof path, x11_socket_fmt, xdisplay);
-    unlink(path);
+    std::array<char, 256> buffer; // avoid allocation in noexcept code
+    auto eos = std::format_to_n(buffer.data(), buffer.size() - 1,
+        x11_lock_fmt, xdisplay).out;
+    *eos = '\0';
+    unlink(buffer.data());
+    eos = std::format_to_n(buffer.data(), buffer.size() - 1,
+        x11_socket_fmt, xdisplay).out;
+    *eos = '\0';
+    unlink(buffer.data());
 }
 
 auto mf::XWaylandSpawner::x11_display() const -> std::string

src/server/report/logging/compositor_report.cpp

@@ -17,6 +17,8 @@
 #include "compositor_report.h"
 #include <mir/logging/logger.h>
 
+#include <format>
+
 using namespace mir::time;
 namespace ml = mir::logging;
 namespace mrl = mir::report::logging;
@@ -43,9 +45,8 @@ mrl::CompositorReport::TimePoint mrl::CompositorReport::now() const
 
 void mrl::CompositorReport::added_display(int width, int height, int x, int y, SubCompositorId id)
 {
-    char msg[128];
-    snprintf(msg, sizeof msg, "Added display %p: %dx%d %+d%+d",
-             id, width, height, x, y);
+    auto const msg = std::format("Added display {}: {}x{} {:+d}{:+d}",
+             static_cast<const void*>(id), width, height, x, y);
     logger->log(ml::Severity::informational, msg, component);
 }
 
@@ -101,13 +102,13 @@ void mrl::CompositorReport::Instance::log(ml::Logger& logger, SubCompositorId id
         long avg_latency_usec = dn ? dl / dn : 0;
         long dt_msec = dt / 1000L;
 
-        char msg[128];
-        snprintf(msg, sizeof msg, "Display %p averaged %ld.%03ld FPS, "
-                 "%ld.%03ld ms/frame, "
-                 "latency %ld.%03ld ms, "
-                 "%ld frames over %ld.%03ld sec, "
-                 "%ld%% bypassed",
-                 id,
+        auto const msg = std::format(
+                 "Display {} averaged {}.{:03d} FPS, "
+                 "{}.{:03d} ms/frame, "
+                 "latency {}.{:03d} ms, "
+                 "{} frames over {}.{:03d} sec, "
+                 "{}% bypassed",
+                 static_cast<const void*>(id),
                  frames_per_1000sec / 1000,
                  frames_per_1000sec % 1000,
                  avg_render_time_usec / 1000,
@@ -156,9 +157,8 @@ void mrl::CompositorReport::finished_frame(SubCompositorId id)
 
     if (inst.bypassed != inst.prev_bypassed || inst.nframes == 1)
     {
-        char msg[128];
-        snprintf(msg, sizeof msg, "Display %p bypass %s",
-                 id, inst.bypassed ? "ON" : "OFF");
+        auto const msg = std::format("Display {} bypass {}",
+                 static_cast<const void*>(id), inst.bypassed ? "ON" : "OFF");
         logger->log(ml::Severity::informational, msg, component);
     }
     inst.prev_bypassed = inst.bypassed;

tests/mir_test_framework/test_wlcs_display_server.cpp

@@ -48,6 +48,7 @@
 #include <sys/eventfd.h>
 
 #include <deque>
+#include <format>
 #include <optional>
 #include <unordered_map>
 
@@ -853,9 +854,8 @@ miral::TestWlcsDisplayServer::TestWlcsDisplayServer(int argc, char const** argv)
 
     add_to_environment("MIR_SERVER_CURSOR", "null");
     add_to_environment("MIR_SERVER_ENABLE_KEY_REPEAT", "false");
-    char buffer[32];
-    snprintf(buffer, sizeof buffer, "wlcs-tests-%d", getpid());
-    add_to_environment("WAYLAND_DISPLAY", buffer);
+    const auto wayland_display = std::format("wlcs-tests-{}", getpid());
+    add_to_environment("WAYLAND_DISPLAY", wayland_display.c_str());
 
     add_server_init([this](mir::Server& server)
         {

tests/performance-tests/test_compositor.cpp

@@ -16,6 +16,7 @@
 
 #include "system_performance_test.h"
 
+#include <format>
 #include <fstream>
 #include <string>
 
@@ -39,9 +40,8 @@ struct CompositorPerformance : SystemPerformanceTest
         const ::testing::TestInfo *const test_info =
                 ::testing::UnitTest::GetInstance()->current_test_info();
 
-        char output_filename[256];
-        snprintf(output_filename, sizeof(output_filename) - 1,
-                 "/tmp/%s_%s_server.log",
+        const auto output_filename = std::format(
+                 "/tmp/{}_{}_server.log",
                  test_info->test_case_name(), test_info->name());
         std::ofstream out{output_filename};
 

tests/performance-tests/test_glmark2-es2.cpp

@@ -27,6 +27,7 @@
 
 #include <cstdio>
 #include <cstdlib>
+#include <format>
 #include <fstream>
 #include <string>
 
@@ -60,12 +61,11 @@ struct AbstractGLMark2Test : testing::Test, mtf::AsyncServerRunner {
         const ::testing::TestInfo *const test_info =
                 ::testing::UnitTest::GetInstance()->current_test_info();
 
-        char output_filename[256];
-        snprintf(output_filename, sizeof(output_filename) - 1,
-                 "/tmp/%s_%s.log",
+        auto const output_filename = std::format("/tmp/{}_{}.log",
                  test_info->test_case_name(), test_info->name());
 
-        printf("Saving GLMark2 detailed results to: %s\n", output_filename);
+        printf("Saving GLMark2 detailed results to: %s\n",
+            output_filename.c_str());
         // ^ Although I would vote to just print them to stdout instead
 
         std::string line;
@@ -158,17 +158,17 @@ struct GLMark2Wayland : AbstractGLMark2Test
 struct HostedGLMark2Wayland : GLMark2Wayland
 {
     static char const* const host_socket;
-    char host_output_filename[256];
+    const char *host_output_filename{nullptr};
     pid_t pid = 0;
 
     HostedGLMark2Wayland()
     {
         const ::testing::TestInfo *const test_info =
             ::testing::UnitTest::GetInstance()->current_test_info();
 
-        snprintf(host_output_filename, sizeof(host_output_filename) - 1,
-                 "/tmp/%s_%s_host.log",
+        s_host_output_filename = std::format("/tmp/{}_{}_host.log",
                  test_info->test_case_name(), test_info->name());
+        host_output_filename = s_host_output_filename.c_str();
 
         if ((pid = fork()))
         {
@@ -224,6 +224,8 @@ struct HostedGLMark2Wayland : GLMark2Wayland
 
     std::vector<char const*> get_host_args() const;
     std::vector<char const*> get_nested_args() const;
+private:
+    std::string s_host_output_filename;
 };
 
 char const* const HostedGLMark2Wayland::host_socket = "GLMark2WaylandHost";