[{"id":14233,"web_url":"https://patchwork.libcamera.org/comment/14233/","msgid":"<CAEmqJPrjrL5rWNmDmu_FCpRhOBQUb3fnyW1jwN8H-Tf0UQgD1Q@mail.gmail.com>","date":"2020-12-13T20:13:06","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: pipeline: raspberrypi:\n\tDon't leak RPiCameraData::sensor_","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your patch.\n\n\nOn Sat, 12 Dec 2020, 6:51 pm Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> The CameraSensor instance stored in RPiCameraData::sensor_ is allocated\n> dynamically and never deleted. Fix the memory leak by storing it in a\n> std::unique_ptr<>.\n>\n> Fixes: 740fd1b62f67 (\"libcamera: pipeline: Raspberry Pi pipeline handler\")\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n\nReviewed-by: Naushir Patuck <naush@raspberrypi.com>\n\n\n---\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 ++++---\n>  1 file changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 439c21ce4566..7a5f5881b9b3 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -7,6 +7,7 @@\n>  #include <algorithm>\n>  #include <assert.h>\n>  #include <fcntl.h>\n> +#include <memory>\n>  #include <mutex>\n>  #include <queue>\n>  #include <sys/mman.h>\n> @@ -134,7 +135,7 @@ class RPiCameraData : public CameraData\n>  {\n>  public:\n>         RPiCameraData(PipelineHandler *pipe)\n> -               : CameraData(pipe), sensor_(nullptr),\n> state_(State::Stopped),\n> +               : CameraData(pipe), state_(State::Stopped),\n>                   supportsFlips_(false), flipsAlterBayerOrder_(false),\n>                   updateScalerCrop_(true), dropFrameCount_(0),\n> ispOutputCount_(0)\n>         {\n> @@ -158,7 +159,7 @@ public:\n>         void handleState();\n>         void applyScalerCrop(const ControlList &controls);\n>\n> -       CameraSensor *sensor_;\n> +       std::unique_ptr<CameraSensor> sensor_;\n>         /* Array of Unicam and ISP device streams and associated\n> buffers/streams. */\n>         RPi::Device<Unicam, 2> unicam_;\n>         RPi::Device<Isp, 4> isp_;\n> @@ -948,7 +949,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n>         /* Identify the sensor. */\n>         for (MediaEntity *entity : unicam_->entities()) {\n>                 if (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {\n> -                       data->sensor_ = new CameraSensor(entity);\n> +                       data->sensor_ =\n> std::make_unique<CameraSensor>(entity);\n>                         break;\n>                 }\n>         }\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n>","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 8B718BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 13 Dec 2020 20:13:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC4F86158D;\n\tSun, 13 Dec 2020 21:13:21 +0100 (CET)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8EECB60322\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Dec 2020 21:13:18 +0100 (CET)","by mail-lf1-x141.google.com with SMTP id l11so25005666lfg.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Dec 2020 12:13:18 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"SiIVf2SX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=xD63E7lcm1PxSOrm7E+cqwCVyalt2m6772zi6ner8jM=;\n\tb=SiIVf2SXhLrO0MvqZKJ/dNyD2N9ZuQlwlrnJibAYbQhbA4y2Eq/nI4qu2TAK66YxGd\n\tw/zs/y5lySCnshQeyoyveuXxqzEs1wqCTvGhp+rIA7yXSfYdzEJ8PK9ZdTwMs3GMEPs8\n\tJwEUT4OL5DD1M4seW/P1thdWvDTz8wnup1bvJ7qNMlgT0tuB2sv8Q1AZIY/W62llAnMZ\n\tqmqbnfCxJpJ1FOh1EkjptofX5GexSAPdP/vvYC4gGAvxTZTrikzuBpswklsej+5tr37h\n\tdvDTC4CC5O8XwCen7WXtsVi+/wjGU6RsqcKxdwExmUFYaepAxnNq98tLILk3lPipbAY7\n\t4k9Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=xD63E7lcm1PxSOrm7E+cqwCVyalt2m6772zi6ner8jM=;\n\tb=fkafpzEGV8YF6pmK7BWHIbTFi5baPzL5vB3qK1Ij5SFgbSsPLV/kTdiWYAoFXOKj3W\n\tG0D94onrGoohHERaiWfo2ewNflyt5xYv6xoiqtKOnE14q9Ug9J1YtLMurHmc0v9KeX14\n\t+xICBUSTZ1GVtZhFgdKkFQQe+ZmI/n+o1emjO5VQBJ3FmWV+bEAs9A9te2ORKjDvmbHI\n\tQ0bvcr75F3EtoDmUuTnVmfNUUa81v5rpJFv+YSVXcWDQi/9tWaOaX+rbDSfPJ4hYS95y\n\t7l+DJiYoNji4/go4/+m9J13IykeDELcIBUj8UMgBPx7ADng04tH32sHhdnVDLh+wXeBV\n\tUw1w==","X-Gm-Message-State":"AOAM532isay1DHTETxYs/mb2BRJW4b2Y86B6pamAprltF6Tw3iF7qnIY\n\tlnyGZoYLG+WboWRL2VZLaNy+Ij/AeuXgOKBImmVnVA==","X-Google-Smtp-Source":"ABdhPJykAADZj8Ifm1LOzqYh4W6HAVOL8keb6oZRHK1EBBlT/nh0CIWiwj0C+Fq1j1P8bbHLTsl91OyWIv5LDw4zxIU=","X-Received":"by 2002:a05:651c:503:: with SMTP id\n\to3mr8628013ljp.253.1607890397797; \n\tSun, 13 Dec 2020 12:13:17 -0800 (PST)","MIME-Version":"1.0","References":"<20201212185116.29611-1-laurent.pinchart@ideasonboard.com>\n\t<20201212185116.29611-2-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20201212185116.29611-2-laurent.pinchart@ideasonboard.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Sun, 13 Dec 2020 20:13:06 +0000","Message-ID":"<CAEmqJPrjrL5rWNmDmu_FCpRhOBQUb3fnyW1jwN8H-Tf0UQgD1Q@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: pipeline: raspberrypi:\n\tDon't leak RPiCameraData::sensor_","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>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"multipart/mixed;\n\tboundary=\"===============2710368757044765931==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14237,"web_url":"https://patchwork.libcamera.org/comment/14237/","msgid":"<20201214033355.GC2095@pyrite.rasen.tech>","date":"2020-12-14T03:33:55","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: pipeline: raspberrypi:\n\tDon't leak RPiCameraData::sensor_","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Sat, Dec 12, 2020 at 08:51:14PM +0200, Laurent Pinchart wrote:\n> The CameraSensor instance stored in RPiCameraData::sensor_ is allocated\n> dynamically and never deleted. Fix the memory leak by storing it in a\n> std::unique_ptr<>.\n> \n> Fixes: 740fd1b62f67 (\"libcamera: pipeline: Raspberry Pi pipeline handler\")\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> ---\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 ++++---\n>  1 file changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 439c21ce4566..7a5f5881b9b3 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -7,6 +7,7 @@\n>  #include <algorithm>\n>  #include <assert.h>\n>  #include <fcntl.h>\n> +#include <memory>\n>  #include <mutex>\n>  #include <queue>\n>  #include <sys/mman.h>\n> @@ -134,7 +135,7 @@ class RPiCameraData : public CameraData\n>  {\n>  public:\n>  \tRPiCameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe), sensor_(nullptr), state_(State::Stopped),\n> +\t\t: CameraData(pipe), state_(State::Stopped),\n>  \t\t  supportsFlips_(false), flipsAlterBayerOrder_(false),\n>  \t\t  updateScalerCrop_(true), dropFrameCount_(0), ispOutputCount_(0)\n>  \t{\n> @@ -158,7 +159,7 @@ public:\n>  \tvoid handleState();\n>  \tvoid applyScalerCrop(const ControlList &controls);\n>  \n> -\tCameraSensor *sensor_;\n> +\tstd::unique_ptr<CameraSensor> sensor_;\n>  \t/* Array of Unicam and ISP device streams and associated buffers/streams. */\n>  \tRPi::Device<Unicam, 2> unicam_;\n>  \tRPi::Device<Isp, 4> isp_;\n> @@ -948,7 +949,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n>  \t/* Identify the sensor. */\n>  \tfor (MediaEntity *entity : unicam_->entities()) {\n>  \t\tif (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {\n> -\t\t\tdata->sensor_ = new CameraSensor(entity);\n> +\t\t\tdata->sensor_ = std::make_unique<CameraSensor>(entity);\n>  \t\t\tbreak;\n>  \t\t}\n>  \t}\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 B4A19BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Dec 2020 03:34:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D79C6158A;\n\tMon, 14 Dec 2020 04:34:05 +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 99ED5600EB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Dec 2020 04:34:03 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E6FEA96;\n\tMon, 14 Dec 2020 04:34:01 +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=\"bIDiYAYG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1607916843;\n\tbh=BrEqVgkEisHYYaTcw39yichZyDWfRWl7WKEvU8qRUIk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bIDiYAYGee6XZyt3q2M0q+efO4SZ0T8Ude65yWRqU7TWCSsZdGeCwXQRAOlR8lXk2\n\t3CEsajNMyLYDcgm4iBh2egOi2xqo9NA+I2ZlwLSAr0yBmWhcjmgDZkhPGHswhwlffb\n\tatpdO2yKVnOpjpBDpOVslNQ2kcYMqNg5/8uzf8hE=","Date":"Mon, 14 Dec 2020 12:33:55 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20201214033355.GC2095@pyrite.rasen.tech>","References":"<20201212185116.29611-1-laurent.pinchart@ideasonboard.com>\n\t<20201212185116.29611-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201212185116.29611-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: pipeline: raspberrypi:\n\tDon't leak RPiCameraData::sensor_","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>","Cc":"libcamera-devel@lists.libcamera.org","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>"}}]