diff --git a/taicpp/src/mplex/mplex_server.cpp b/taicpp/src/mplex/mplex_server.cpp index b397405..eea1dab 100644 --- a/taicpp/src/mplex/mplex_server.cpp +++ b/taicpp/src/mplex/mplex_server.cpp @@ -291,7 +291,7 @@ void MplexServer::add_timer(double deadline, std::function callback) { // ---- buffers and copy --------------------------------------------------------------------- -std::string MplexServer::add_buffer(const std::string& text, const std::optional& name_in) { +std::string MplexServer::add_buffer(std::string text, std::optional name_in) { std::string name = name_in ? *name_in : "buffer" + std::to_string(next_id("buffer")); buffers.erase(std::remove_if(buffers.begin(), buffers.end(), [&](const MplexBuffer& b) { return b.name == name; }), buffers.end()); @@ -540,7 +540,7 @@ std::shared_ptr MplexServer::split_pane(const std::shared_ptr& pane) { +void MplexServer::discard_pane(std::shared_ptr pane) { pane->close(); panes.erase(pane->id); if (pane->pid) { @@ -559,7 +559,7 @@ void MplexServer::select_pane(std::shared_ptr window, std::shared_p mark_dirty(window); } -void MplexServer::kill_pane(const std::shared_ptr& pane) { +void MplexServer::kill_pane(std::shared_ptr pane) { auto window = pane->window(); if (!window) return; if (window->panes().size() == 1) { @@ -589,7 +589,7 @@ void MplexServer::kill_pane(const std::shared_ptr& pane) { mark_dirty(window); } -void MplexServer::kill_window(const std::shared_ptr& window) { +void MplexServer::kill_window(std::shared_ptr window) { auto session = window->session(); if (!session) throw std::runtime_error("'NoneType' object has no attribute 'index_of'"); auto index = session->index_of(window); @@ -632,7 +632,7 @@ void MplexServer::renumber(const std::shared_ptr& session) { session->current = current ? session->index_of(current) : std::nullopt; } -void MplexServer::kill_session(const std::shared_ptr& session) { +void MplexServer::kill_session(std::shared_ptr session) { for (const auto& window : session->windows.values()) { for (const auto& pane : window->panes()) { if (marked == pane) marked = nullptr; diff --git a/taicpp/src/mplex/mplex_server.h b/taicpp/src/mplex/mplex_server.h index 39451bf..b48dcf7 100644 --- a/taicpp/src/mplex/mplex_server.h +++ b/taicpp/src/mplex/mplex_server.h @@ -143,7 +143,7 @@ class MplexServer { void add_timer(double deadline, std::function callback); // ---- buffers and copy - std::string add_buffer(const std::string& text, const std::optional& name = std::nullopt); + std::string add_buffer(std::string text, std::optional name = std::nullopt); // by value: text may alias a buffer that is erased std::string top_buffer_text() const; MplexBuffer& find_buffer(const std::string& name); // throws MplexError("no buffers"/"unknown buffer: x") void run_shell_input(const std::string& command, const std::string& text); @@ -171,12 +171,12 @@ class MplexServer { std::shared_ptr split_pane(const std::shared_ptr& window, const std::shared_ptr& target, const std::string& kind, bool before, const std::vector& words, const std::string& cwd, const MplexPane::EnvList* env_extra, int size, bool select); - void discard_pane(const std::shared_ptr& pane); + void discard_pane(std::shared_ptr pane); void select_pane(std::shared_ptr window, std::shared_ptr pane); // by value: pane may alias window->last_pane - void kill_pane(const std::shared_ptr& pane); - void kill_window(const std::shared_ptr& window); + void kill_pane(std::shared_ptr pane); // by value: arguments may alias members that are reset + void kill_window(std::shared_ptr window); void renumber(const std::shared_ptr& session); - void kill_session(const std::shared_ptr& session); + void kill_session(std::shared_ptr session); void attach_client(const std::shared_ptr& client, const std::shared_ptr& session); void detach_client(const std::shared_ptr& client, const std::string& text = "", const std::string& kind = "D"); diff --git a/taicpp/tests/mplex_regress_test.cpp b/taicpp/tests/mplex_regress_test.cpp new file mode 100644 index 0000000..b340915 --- /dev/null +++ b/taicpp/tests/mplex_regress_test.cpp @@ -0,0 +1,80 @@ +// Regression tests for C++-vs-Python semantic traps found by the audit (G2): const-reference +// aliasing of members, killed objects reachable from the argument, buffer self-reference. +// Links all of src/mplex and src/plexer/plexer_flow.cpp. Plain main + asserts. +#include + +#include +#include +#include +#include + +#include "mplex_server.h" + +using namespace detz::mplex; + +static int failures = 0; +static int checks = 0; +static void check(bool ok, const char* what) { + ++checks; + if (!ok) { + ++failures; + std::cerr << "FAIL: " << what << "\n"; + } +} + +int main() { + char tmpl[] = "/tmp/mplex-regress-XXXXXX"; + std::string home = mkdtemp(tmpl); + { + MplexServer server(home); + MplexPane::EnvList env = {{"PATH", "/usr/bin:/bin"}, {"HOME", home}}; + auto session = server.create_session("s", "/", env, {"/bin/sh"}, "w", {80, 24}); + auto window = session->window(); + auto p0 = window->active; + + // set_active / select_pane called with window->last_pane (member aliasing) + auto p1 = server.split_pane(window, p0, "lr", false, {"/bin/sh"}, "/", nullptr, -1, true); + auto p2 = server.split_pane(window, p1, "tb", false, {"/bin/sh"}, "/", nullptr, -1, true); + check(window->active == p2 && window->last_pane == p1, "split leaves last_pane = previous active"); + server.select_pane(window, window->last_pane); + check(window->active == p1, "select_pane(window, window->last_pane) activates the last pane"); + check(window->last_pane == p2, "... and records the old active as last_pane"); + window->set_active(window->last_pane); + check(window->active == p2 && window->last_pane == p1, "set_active(window->last_pane) swaps"); + + // add_buffer with text aliasing a buffer that gets replaced + server.add_buffer("alpha", std::string("b")); + server.add_buffer(server.buffers[0].data, std::string("b")); + check(server.buffers.size() == 1 && server.buffers[0].data == "alpha", "add_buffer(buffers[0].data, same name)"); + + // kill_pane with the argument aliasing server.marked (cleared inside) + server.marked = p1; + server.kill_pane(server.marked); + check(!server.marked && !server.panes.contains(p1->id), "kill_pane(server.marked)"); + check(window->panes().size() == 2, "window keeps the other panes"); + + // kill_pane with the argument aliasing window->active + server.kill_pane(window->active); + check(window->panes().size() == 1 && window->active, "kill_pane(window->active) leaves an active pane"); + + // kill_session with the argument aliasing an element of server.sessions + auto other = server.create_session("t", "/", env, {"/bin/sh"}, "w", {80, 24}); + int other_id = other->id; + other.reset(); + server.kill_session(server.sessions.items()[1].second); + check(!server.sessions.contains(other_id) && server.sessions.size() == 1, "kill_session(sessions[...])"); + + // kill_window with the argument aliasing an element of server.windows + auto w2 = server.create_window(session, "x", "/", {"/bin/sh"}, nullptr, 5, false); + int w2id = w2->id; + w2.reset(); + server.kill_window(server.windows.items().back().second); + check(!server.windows.contains(w2id) && session->windows.size() == 1, "kill_window(windows[...])"); + + server.kill_session(session); + check(server.sessions.empty() && server.panes.empty(), "everything gone"); + } + std::filesystem::remove_all(home); + std::printf("mplex_regress_test: %d checks, %d failures\n", checks, failures); + return failures == 0 ? 0 : 1; +}