Skip to content

Commit 552d77b

Browse files
committed
Fix Tier 3 code review findings
1 parent 30e176f commit 552d77b

6 files changed

Lines changed: 40 additions & 7 deletions

File tree

plugin_arch/DynamicPluginAdapter.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ class DynamicPluginAdapter : public IPlugin,
112112
return lib_->has(func_name);
113113
}
114114

115+
// WARNING: function_count and functions come from the plugin — the framework
116+
// cannot validate that function_count matches the actual array length.
117+
// Callers iterating functions must not trust function_count blindly if the
118+
// plugin source is untrusted.
115119
[[nodiscard]] const PluginDescriptor& descriptor() const { return *desc_; }
116120

117121
[[nodiscard]] const std::shared_ptr<DynamicLibrary>& library() const {

plugin_arch/EventBus.hpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@ namespace plugin_arch {
2929

3030
class EventBus {
3131
public:
32+
EventBus() = default;
33+
~EventBus() = default;
34+
35+
EventBus(const EventBus&) = delete;
36+
EventBus& operator=(const EventBus&) = delete;
37+
EventBus(EventBus&&) = default;
38+
EventBus& operator=(EventBus&&) = default;
39+
3240
using Handler = std::function<void(const std::string& topic,
3341
const std::string& payload)>;
3442
using SubscriptionId = std::size_t;
@@ -117,7 +125,7 @@ class EventBus {
117125
}
118126
}
119127

120-
// --- Vetoable API (B5) ---
128+
// --- Vetoable API ---
121129

122130
// Subscribe a veto handler. Returns true to allow, false to veto.
123131
// Higher priority = checked first.

plugin_arch/HotPluginLoader.hpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#pragma once
99

1010
#include <filesystem>
11-
#include <iostream>
1211
#include <memory>
1312
#include <string>
1413

@@ -47,9 +46,7 @@ class HotPluginLoader {
4746
try {
4847
reload();
4948
return true;
50-
} catch (const std::exception& e) {
51-
std::cerr << "[HotPluginLoader] reload failed: " << e.what()
52-
<< " — keeping current version\n";
49+
} catch (const std::exception&) {
5350
last_modified_ = mtime; // avoid retrying every poll
5451
return false;
5552
}

plugin_arch/PluginManager.hpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,14 @@ inline void apply_config_defaults(IPlugin* instance, PluginConfig& config) {
304304

305305
class PluginManager {
306306
public:
307+
PluginManager() = default;
308+
~PluginManager() = default;
309+
310+
PluginManager(const PluginManager&) = delete;
311+
PluginManager& operator=(const PluginManager&) = delete;
312+
PluginManager(PluginManager&&) = delete;
313+
PluginManager& operator=(PluginManager&&) = delete;
314+
307315
// Per-plugin configuration, keyed by plugin name.
308316
using ConfigMap = std::unordered_map<std::string, PluginConfig>;
309317

@@ -869,7 +877,7 @@ class PluginManager {
869877
// after built-in mixin injection (configure, service locator, event bus) and
870878
// before on_init(). Allows hosts to wire custom mixins without modifying
871879
// PluginManager source.
872-
using MixinWirer = std::function<void(std::shared_ptr<IPlugin>&)>;
880+
using MixinWirer = std::function<void(const std::shared_ptr<IPlugin>&)>;
873881

874882
void add_mixin_wirer(MixinWirer wirer) {
875883
if (wirer) custom_wirers_.push_back(std::move(wirer));
@@ -1213,7 +1221,7 @@ class PluginManager {
12131221

12141222
// Wire all opt-in mixins on an instance.
12151223
// Order: validate+configure → inject services → inject events → custom wirers → init.
1216-
void wire_instance(std::shared_ptr<IPlugin>& instance,
1224+
void wire_instance(const std::shared_ptr<IPlugin>& instance,
12171225
const PluginEntry& entry, const ConfigMap& config_map,
12181226
bool skip_validation = false) {
12191227
auto resolved = resolve_plugin_config(

plugin_arch/PluginRegistry.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ struct ErrorRecord {
5151

5252
class PluginRegistry {
5353
public:
54+
PluginRegistry() = default;
55+
~PluginRegistry() = default;
56+
57+
PluginRegistry(const PluginRegistry&) = delete;
58+
PluginRegistry& operator=(const PluginRegistry&) = delete;
59+
PluginRegistry(PluginRegistry&&) = default;
60+
PluginRegistry& operator=(PluginRegistry&&) = default;
61+
5462
// Scan a directory for shared libraries and probe each one for metadata.
5563
// Appends to existing entries, so you can scan multiple directories.
5664
// Returns the number of new plugins discovered in this call.

plugin_arch/ServiceLocator.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ namespace plugin_arch {
1919

2020
class ServiceLocator {
2121
public:
22+
ServiceLocator() = default;
23+
~ServiceLocator() = default;
24+
25+
ServiceLocator(const ServiceLocator&) = delete;
26+
ServiceLocator& operator=(const ServiceLocator&) = delete;
27+
ServiceLocator(ServiceLocator&&) = default;
28+
ServiceLocator& operator=(ServiceLocator&&) = default;
29+
2230
// Register a loaded plugin as a service.
2331
// The locator holds a weak reference — it does not extend the plugin's lifetime.
2432
void add(const std::shared_ptr<IPlugin>& service) {

0 commit comments

Comments
 (0)