Patch Detail
Show a patch.
GET /api/patches/11568/?format=api
{ "id": 11568, "url": "https://patchwork.libcamera.org/api/patches/11568/?format=api", "web_url": "https://patchwork.libcamera.org/patch/11568/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20210312061131.854849-9-kieran.bingham@ideasonboard.com>", "date": "2021-03-12T06:11:31", "name": "[libcamera-devel,8/8,RFC-Only] libcamera: request: A request canary", "commit_ref": null, "pull_url": null, "state": "changes-requested", "archived": false, "hash": "e1f245dcce4e24299f5ee0463c370b2e4e2b9cc5", "submitter": { "id": 4, "url": "https://patchwork.libcamera.org/api/people/4/?format=api", "name": "Kieran Bingham", "email": "kieran.bingham@ideasonboard.com" }, "delegate": { "id": 11, "url": "https://patchwork.libcamera.org/api/users/11/?format=api", "username": "kbingham", "first_name": "Kieran", "last_name": "Bingham", "email": "kieran.bingham@ideasonboard.com" }, "mbox": "https://patchwork.libcamera.org/patch/11568/mbox/", "series": [ { "id": 1786, "url": "https://patchwork.libcamera.org/api/series/1786/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1786", "date": "2021-03-12T06:11:23", "name": "IPU3 Debug improvements", "version": 1, "mbox": "https://patchwork.libcamera.org/series/1786/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/11568/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/11568/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 30C57BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Mar 2021 06:11:45 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE4F168C84;\n\tFri, 12 Mar 2021 07:11:44 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 01B0468C6B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Mar 2021 07:11:39 +0100 (CET)", "from localhost.localdomain\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 8C912F95;\n\tFri, 12 Mar 2021 07:11:38 +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=\"h0290R9X\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615529498;\n\tbh=CXpckEd6t9K6vgSZfAjDek218hD66ksQ8YyosdKLQ20=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=h0290R9XE7bqzekZ+WghhU1f7gnFJ2Iqx5hpFGterajV/L/9nSJO/0v0Imo0KTVX3\n\th3wXH5U76ZV7XPDdNfrILkfVO6vcccdeUuR4fPu5ccHAH738HnRpfuG/mBD9D+H+5d\n\tysLIZrTTvvpgMbgCyUzlk1Z+BhHWoTamCxIq3BmE=", "From": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "Date": "Fri, 12 Mar 2021 06:11:31 +0000", "Message-Id": "<20210312061131.854849-9-kieran.bingham@ideasonboard.com>", "X-Mailer": "git-send-email 2.25.1", "In-Reply-To": "<20210312061131.854849-1-kieran.bingham@ideasonboard.com>", "References": "<20210312061131.854849-1-kieran.bingham@ideasonboard.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH 8/8] [RFC-Only] libcamera: request: A\n\trequest canary", "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>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Request objects are created and owned by the application, but are of\ncourse utilised widely throughout the internals of libcamera.\n\nIf the application free's the requests while they are still active\nwithin libcamera a use after free will occur. While this can be trapped\nby tools such as valgrind, given the importance of this object and the\nrelationship of external ownership, it may have some value to provide\nDebug build (disabled at Release build) assertions on the condition of\nthese objects.\n\nMake sure the request fails an assertion immediately if used after free.\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n---\nRFC only, as I was going to dispose of this patch. We added it while\ndebugging the IPU3 stability, and it may prove useful to catch errors if\nrequests are used after they are released.\n\nHowever this may be redundant as pipeline handlers should guarantee that\nrequests are fully completed when they stop(). Of course the IPU3 wasn't\nmeeting that requirement, and we do not have a specific assertion that\nvalidates that requirement on all pipeline handlers. So perhaps this\ncanary might serve as a beneficial guard?\n\nIf not, at least posting it will mean it can be used in the future if it\ncomes up again, or the concept could be applied to other objects if\nappropriate.\n\n include/libcamera/request.h | 2 ++\n src/libcamera/request.cpp | 15 +++++++++++++++\n 2 files changed, 17 insertions(+)", "diff": "diff --git a/include/libcamera/request.h b/include/libcamera/request.h\nindex 59d7f4bac0d2..fc56d63c8c67 100644\n--- a/include/libcamera/request.h\n+++ b/include/libcamera/request.h\n@@ -78,6 +78,8 @@ private:\n \tconst uint64_t cookie_;\n \tStatus status_;\n \tbool cancelled_;\n+\n+\tint canary_;\n };\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex 12c2e7d425f9..83169a11e1e5 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -18,6 +18,8 @@\n #include \"libcamera/internal/log.h\"\n #include \"libcamera/internal/tracepoints.h\"\n \n+#define REQUEST_CANARY 0x1F2E3D4C\n+\n /**\n * \\file request.h\n * \\brief Describes a frame capture request to be processed by a camera\n@@ -89,6 +91,8 @@ Request::Request(Camera *camera, uint64_t cookie)\n \n \tLIBCAMERA_TRACEPOINT(request_construct, this);\n \n+\tcanary_ = REQUEST_CANARY;\n+\n \tLOG(Request, Debug) << \"Created request - cookie: \" << cookie_;\n }\n \n@@ -99,6 +103,8 @@ Request::~Request()\n \tdelete metadata_;\n \tdelete controls_;\n \tdelete validator_;\n+\n+\tcanary_ = 0;\n }\n \n /**\n@@ -113,6 +119,8 @@ Request::~Request()\n */\n void Request::reuse(ReuseFlag flags)\n {\n+\tASSERT(canary_ == REQUEST_CANARY);\n+\n \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n \n \tpending_.clear();\n@@ -176,6 +184,8 @@ void Request::reuse(ReuseFlag flags)\n */\n int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)\n {\n+\tASSERT(canary_ == REQUEST_CANARY);\n+\n \tif (!stream) {\n \t\tLOG(Request, Error) << \"Invalid stream reference\";\n \t\treturn -EINVAL;\n@@ -211,6 +221,8 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)\n */\n FrameBuffer *Request::findBuffer(const Stream *stream) const\n {\n+\tASSERT(canary_ == REQUEST_CANARY);\n+\n \tconst auto it = bufferMap_.find(stream);\n \tif (it == bufferMap_.end())\n \t\treturn nullptr;\n@@ -262,6 +274,7 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const\n */\n void Request::complete()\n {\n+\tASSERT(canary_ == REQUEST_CANARY);\n \tASSERT(status_ == RequestPending);\n \tASSERT(!hasPendingBuffers());\n \n@@ -289,6 +302,8 @@ void Request::complete()\n */\n bool Request::completeBuffer(FrameBuffer *buffer)\n {\n+\tASSERT(canary_ == REQUEST_CANARY);\n+\n \tLIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer);\n \n \tint ret = pending_.erase(buffer);\n", "prefixes": [ "libcamera-devel", "8/8", "RFC-Only" ] }