Patch Detail
Show a patch.
GET /api/1.1/patches/15319/?format=api
{ "id": 15319, "url": "https://patchwork.libcamera.org/api/1.1/patches/15319/?format=api", "web_url": "https://patchwork.libcamera.org/patch/15319/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20220201123803.281397-3-kieran.bingham@ideasonboard.com>", "date": "2022-02-01T12:38:03", "name": "[libcamera-devel,v4,2/2] libcamera: base: object: Prevent the same signal being connected more than once", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "4871157b4ef77f85cc1fc0c9739683345cd9f06b", "submitter": { "id": 4, "url": "https://patchwork.libcamera.org/api/1.1/people/4/?format=api", "name": "Kieran Bingham", "email": "kieran.bingham@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/15319/mbox/", "series": [ { "id": 2908, "url": "https://patchwork.libcamera.org/api/1.1/series/2908/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2908", "date": "2022-02-01T12:38:01", "name": "libcamera: pipeline_handler: Register requests", "version": 4, "mbox": "https://patchwork.libcamera.org/series/2908/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/15319/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/15319/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 16301BDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 1 Feb 2022 12:38:10 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 19085609EF;\n\tTue, 1 Feb 2022 13:38:09 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7AC83609B9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 1 Feb 2022 13:38:07 +0100 (CET)", "from Monstersaurus.ksquared.org.uk.beta.tailscale.net\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0FE9B332;\n\tTue, 1 Feb 2022 13:38:07 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CB1vrEhP\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1643719087;\n\tbh=wrcejpDfTZbnVK1qkJmQxFCVFQ8gXidyzz9I3ZPJeuE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=CB1vrEhPVK2qzuf94RFPKrYbTsTUTKGSHVSQnAh0ttblrH7zXku7FjJS8Gj85aHau\n\tO/Ng/Q1VakOCAVyvGDkXI0WiLf78KPW30PEB6dlw4zFd8QiYsZuSquZNw/5Hal+5UC\n\tN1WX8M2yCS4qR0GzrJ9oFWYyHDrOrX3DpSJ2lxio=", "From": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "Date": "Tue, 1 Feb 2022 12:38:03 +0000", "Message-Id": "<20220201123803.281397-3-kieran.bingham@ideasonboard.com>", "X-Mailer": "git-send-email 2.32.0", "In-Reply-To": "<20220201123803.281397-1-kieran.bingham@ideasonboard.com>", "References": "<20220201123803.281397-1-kieran.bingham@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v4 2/2] libcamera: base: object: Prevent\n\tthe same signal being connected more than once", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Objects are not expected to be connected to the same signal more than\nonce. Doing so likely indicates a bug in the code, and can be\nhighlighted in debug builds with an assert that performs a lookup on the\nsignals_ list.\n\nWhile it is possible to allow the implementation to let objects connect\nto a specific signal multiple times, there are no expected use cases for\nthis in libcamera and this behaviour is restricted to favour defensive\nprogramming by raising an error when this occurs.\n\nRemove the support in the test framework which uses multiple Signal\nconnections on the same object, and update the test to use a second\nSignal.\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n---\n src/libcamera/base/object.cpp | 12 ++++++++++++\n src/libcamera/base/signal.cpp | 7 ++++---\n test/signal.cpp | 17 +++++++++++------\n 3 files changed, 27 insertions(+), 9 deletions(-)", "diff": "diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\nindex 3f28768e48f8..13e8ee99caac 100644\n--- a/src/libcamera/base/object.cpp\n+++ b/src/libcamera/base/object.cpp\n@@ -47,6 +47,12 @@ LOG_DEFINE_CATEGORY(Object)\n * object's thread, regardless of whether the signal is emitted in the same or\n * in another thread.\n *\n+ * Objects can be connected to multiple signals, buy they can only be connected\n+ * to each signal once. While it is possible to allow the implementation to let\n+ * objects connect to a specific signal multiple times, there are no expected\n+ * use cases for this in libcamera and this behaviour is restricted to favour\n+ * defensive programming by raising an error when this occurs.\n+ *\n * \\sa Message, Signal, Thread\n */\n \n@@ -284,6 +290,12 @@ void Object::notifyThreadMove()\n \n void Object::connect(SignalBase *signal)\n {\n+\t/*\n+\t * Connecting the same signal to an object multiple times is not\n+\t * supported.\n+\t */\n+\tASSERT(std::find(signals_.begin(), signals_.end(), signal) == signals_.end());\n+\n \tsignals_.push_back(signal);\n }\n \ndiff --git a/src/libcamera/base/signal.cpp b/src/libcamera/base/signal.cpp\nindex 9df45d079a42..a46386a05abf 100644\n--- a/src/libcamera/base/signal.cpp\n+++ b/src/libcamera/base/signal.cpp\n@@ -93,9 +93,7 @@ SignalBase::SlotList SignalBase::slots()\n * Connecting a signal to a slot results in the slot being called with the\n * arguments passed to the emit() function when the signal is emitted. Multiple\n * slots can be connected to the same signal, and multiple signals can connected\n- * to the same slot. Duplicate connections between a signal and a slot are\n- * allowed and result in the slot being called multiple times for the same\n- * signal emission.\n+ * to the same slot.\n *\n * When a slot belongs to an instance of the Object class, the slot is called\n * in the context of the thread that the object is bound to. If the signal is\n@@ -105,6 +103,9 @@ SignalBase::SlotList SignalBase::slots()\n * loop, after the Signal::emit() function returns, with a copy of the signal's\n * arguments. The emitter shall thus ensure that any pointer or reference\n * passed through the signal will remain valid after the signal is emitted.\n+ *\n+ * Duplicate connections between a signal and a slot are not expected and use of\n+ * the Object class to manage signals will enforce this restriction.\n */\n \n /**\ndiff --git a/test/signal.cpp b/test/signal.cpp\nindex fcf2def18df4..48f905348ca3 100644\n--- a/test/signal.cpp\n+++ b/test/signal.cpp\n@@ -212,17 +212,19 @@ protected:\n \t\t/* ----------------- Signal -> Object tests ----------------- */\n \n \t\t/*\n-\t\t * Test automatic disconnection on object deletion. Connect the\n-\t\t * slot twice to ensure all instances are disconnected.\n+\t\t * Test automatic disconnection on object deletion. Connect two\n+\t\t * signals to ensure all instances are disconnected.\n \t\t */\n \t\tsignalVoid_.disconnect();\n+\t\tsignalVoid2_.disconnect();\n \n \t\tSlotObject *slotObject = new SlotObject();\n \t\tsignalVoid_.connect(slotObject, &SlotObject::slot);\n-\t\tsignalVoid_.connect(slotObject, &SlotObject::slot);\n+\t\tsignalVoid2_.connect(slotObject, &SlotObject::slot);\n \t\tdelete slotObject;\n \t\tvalueStatic_ = 0;\n \t\tsignalVoid_.emit();\n+\t\tsignalVoid2_.emit();\n \t\tif (valueStatic_ != 0) {\n \t\t\tcout << \"Signal disconnection on object deletion test failed\" << endl;\n \t\t\treturn TestFail;\n@@ -298,17 +300,19 @@ protected:\n \t\t/* --------- Signal -> Object (multiple inheritance) -------- */\n \n \t\t/*\n-\t\t * Test automatic disconnection on object deletion. Connect the\n-\t\t * slot twice to ensure all instances are disconnected.\n+\t\t * Test automatic disconnection on object deletion. connect two\n+\t\t * signals to ensure all instances are disconnected.\n \t\t */\n \t\tsignalVoid_.disconnect();\n+\t\tsignalVoid2_.disconnect();\n \n \t\tSlotMulti *slotMulti = new SlotMulti();\n \t\tsignalVoid_.connect(slotMulti, &SlotMulti::slot);\n-\t\tsignalVoid_.connect(slotMulti, &SlotMulti::slot);\n+\t\tsignalVoid2_.connect(slotMulti, &SlotMulti::slot);\n \t\tdelete slotMulti;\n \t\tvalueStatic_ = 0;\n \t\tsignalVoid_.emit();\n+\t\tsignalVoid2_.emit();\n \t\tif (valueStatic_ != 0) {\n \t\t\tcout << \"Signal disconnection on object deletion test failed\" << endl;\n \t\t\treturn TestFail;\n@@ -345,6 +349,7 @@ protected:\n \n private:\n \tSignal<> signalVoid_;\n+\tSignal<> signalVoid2_;\n \tSignal<int> signalInt_;\n \tSignal<int, const std::string &> signalMultiArgs_;\n \n", "prefixes": [ "libcamera-devel", "v4", "2/2" ] }