From d7156e495718a0c8b5f7cd443bab3e47590c6162 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 16 Oct 2024 11:50:54 +0100 Subject: [PATCH 1/3] Fix the inotify event handling in ReloadingYamlFileDisplayConfig::auto_reload() --- src/miral/static_display_config.cpp | 34 +++++++++++++++++++---------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/miral/static_display_config.cpp b/src/miral/static_display_config.cpp index 7812aae4d93..fd71be171d8 100644 --- a/src/miral/static_display_config.cpp +++ b/src/miral/static_display_config.cpp @@ -644,20 +644,29 @@ void miral::ReloadingYamlFileDisplayConfig::auto_reload() if (auto const ml = the_main_loop()) { ml->register_fd_handler({icf.value()}, this, [icf=icf.value(), this] (int) + { + static size_t const sizeof_inotify_event = sizeof(inotify_event); + + // A union ensures buffer is aligned for inotify_event + union { - union - { - inotify_event event; - char buffer[sizeof(inotify_event) + NAME_MAX + 1]; - } inotify_buffer; + char buffer[sizeof_inotify_event + NAME_MAX + 1]; + inotify_event unused [[maybe_unused]]; + } inotify_magic; - if (read(icf, &inotify_buffer, sizeof(inotify_buffer)) < static_cast(sizeof(inotify_event))) - return; + auto const readsize = read(icf, &inotify_magic, sizeof(inotify_magic)); + if (readsize < static_cast(sizeof_inotify_event)) + return; - if (inotify_buffer.event.mask & (IN_CLOSE_WRITE | IN_CREATE | IN_MOVED_TO)) + auto raw_buffer = inotify_magic.buffer; + while (raw_buffer != inotify_magic.buffer + readsize) + { + // This is safe because inotify_magic.buffer is aligned and event.len includes padding for alignment + auto& event = reinterpret_cast(*raw_buffer); + if (event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO)) try { - if (inotify_buffer.event.name == basename) + if (event.name == basename) { std::lock_guard lock{mutex}; auto const& filename = config_path_.value() + "/" + basename; @@ -671,7 +680,7 @@ void miral::ReloadingYamlFileDisplayConfig::auto_reload() mir::log_warning("Failed to open display configuration: %s", filename.c_str()); } } - else if (inotify_buffer.event.name == basename + layout_suffix) + else if (event.name == basename + layout_suffix) { check_for_layout_override(); } @@ -691,7 +700,10 @@ void miral::ReloadingYamlFileDisplayConfig::auto_reload() { mir::log_warning("Failed to reload display configuration: %s", except.what()); } - }); + + raw_buffer += sizeof_inotify_event+event.len; + } + }); } } } From b87870356ff85bd9f3abdf5beece3a6a4eaa371f Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 17 Oct 2024 09:42:27 +0100 Subject: [PATCH 2/3] `alignas` is better magic --- src/miral/config_file.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/miral/config_file.cpp b/src/miral/config_file.cpp index 914bfd0785f..4253a9bc7f4 100644 --- a/src/miral/config_file.cpp +++ b/src/miral/config_file.cpp @@ -188,24 +188,19 @@ void Watcher::handler(int) const { static size_t const sizeof_inotify_event = sizeof(inotify_event); - // A union ensures buffer is aligned for inotify_event - union - { - char buffer[sizeof_inotify_event + NAME_MAX + 1]; - inotify_event unused [[maybe_unused]]; - } inotify_magic; + alignas(inotify_event) char buffer[sizeof(inotify_event) + NAME_MAX + 1]; - auto const readsize = read(inotify_fd, &inotify_magic, sizeof(inotify_magic)); + auto const readsize = read(inotify_fd, buffer, sizeof(buffer)); if (readsize < static_cast(sizeof_inotify_event)) { return; } - auto raw_buffer = inotify_magic.buffer; - while (raw_buffer != inotify_magic.buffer + readsize) + auto raw_buffer = buffer; + while (raw_buffer != buffer + readsize) { // This is safe because inotify_magic.buffer is aligned and event.len includes padding for alignment - auto& event = reinterpret_cast(*raw_buffer); + auto const& event = reinterpret_cast(*raw_buffer); if (event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO) && event.wd == directory_watch_descriptor.value()) try { From ea732467fdeb1e055815c64f8f0a154035d98713 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 21 Oct 2024 04:28:10 +0100 Subject: [PATCH 3/3] `alignas` is better magic --- src/miral/static_display_config.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/miral/static_display_config.cpp b/src/miral/static_display_config.cpp index fd71be171d8..599cd491919 100644 --- a/src/miral/static_display_config.cpp +++ b/src/miral/static_display_config.cpp @@ -647,21 +647,16 @@ void miral::ReloadingYamlFileDisplayConfig::auto_reload() { static size_t const sizeof_inotify_event = sizeof(inotify_event); - // A union ensures buffer is aligned for inotify_event - union - { - char buffer[sizeof_inotify_event + NAME_MAX + 1]; - inotify_event unused [[maybe_unused]]; - } inotify_magic; + alignas(inotify_event) char buffer[sizeof(inotify_event) + NAME_MAX + 1]; - auto const readsize = read(icf, &inotify_magic, sizeof(inotify_magic)); + auto const readsize = read(icf, buffer, sizeof(buffer)); if (readsize < static_cast(sizeof_inotify_event)) return; - auto raw_buffer = inotify_magic.buffer; - while (raw_buffer != inotify_magic.buffer + readsize) + auto raw_buffer = buffer; + while (raw_buffer != buffer + readsize) { - // This is safe because inotify_magic.buffer is aligned and event.len includes padding for alignment + // This is safe because buffer is aligned and event.len includes padding for alignment auto& event = reinterpret_cast(*raw_buffer); if (event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO)) try