From 901d9850805f7b6ec3b64232356585a776ddc623 Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Mon, 11 May 2020 22:54:26 +0200
Subject: [PATCH] Fix potential issue with plugins reporting size

I thought this was a problem for a plugin when it was not, but I can
still see this being a source of segfaults.
---
 CHANGELOG.md                 |  1 +
 src/common/events.h          | 24 ++++++++++++++++++------
 src/plugin/plugin-bridge.cpp |  7 ++++++-
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 22e44e90..44d39014 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 
 - Added a workaround for the compilation issues under Wine 5.7 and above as
   caused by [Wine bug 49138](https://bugs.winehq.org/show_bug.cgi?id=49138).
+- Fixed potential issue with plugins not returning their editor size.
 
 ## [1.1.2] - 2020-05-09
 
diff --git a/src/common/events.h b/src/common/events.h
index 0910d806..d1d06429 100644
--- a/src/common/events.h
+++ b/src/common/events.h
@@ -253,10 +253,14 @@ template <typename F>
 auto passthrough_event(AEffect* plugin, F callback) {
     return [=](Event& event) -> EventResult {
         // This buffer is used to write strings and small objects to. We'll
-        // initialize it with a single null to prevent it from being read as
-        // some arbitrary C-style string.
+        // initialize the beginning with null values to both prevent it from
+        // being read as some arbitrary C-style string, and to make sure that
+        // `*static_cast<void**>(string_buffer.data)` will be a null pointer if
+        // the plugin is supposed to write a pointer there but doesn't (such as
+        // with `effEditGetRect`/`WantsVstRect`).
         std::array<char, max_string_length> string_buffer;
-        string_buffer[0] = 0;
+        std::fill(string_buffer.begin(), string_buffer.begin() + sizeof(size_t),
+                  0);
 
         auto read_payload_fn = overload{
             [&](const std::nullptr_t&) -> void* { return nullptr; },
@@ -345,9 +349,17 @@ auto passthrough_event(AEffect* plugin, F callback) {
             },
             [&](WantsAEffectUpdate&) -> EventResultPayload { return *plugin; },
             [&](WantsVstRect&) -> EventResultPayload {
-                // The plugin has written a pointer to a VstRect struct into the
-                // data poitner
-                return **static_cast<VstRect**>(data);
+                // The plugin should have written a pointer to a VstRect struct
+                // into the data pointer. I haven't seen this fail yet, but
+                // since some hosts will call `effEditGetRect()` before
+                // `effEditOpen()` I can assume there are plugins that don't
+                // handle this correctly.
+                VstRect* editor_rect = *static_cast<VstRect**>(data);
+                if (editor_rect == nullptr) {
+                    return nullptr;
+                }
+
+                return *editor_rect;
             },
             [&](WantsVstTimeInfo&) -> EventResultPayload {
                 // Not sure why the VST API has twenty different ways of
diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp
index db30d4f8..de236faf 100644
--- a/src/plugin/plugin-bridge.cpp
+++ b/src/plugin/plugin-bridge.cpp
@@ -342,7 +342,12 @@ class DispatchDataConverter : DefaultDataConverter {
                 update_aeffect(plugin, updated_plugin);
             } break;
             case effEditGetRect: {
-                // Write back the (hopefully) updated editor dimensions
+                // Either the plugin will have returned (a pointer to) their
+                // editor dimensions, or they will not have written anything.
+                if (std::holds_alternative<std::nullptr_t>(response.payload)) {
+                    return;
+                }
+
                 const auto new_rect = std::get<VstRect>(response.payload);
                 rect = new_rect;