[{"id":17363,"web_url":"https://patchwork.libcamera.org/comment/17363/","msgid":"","date":"2021-06-01T19:27:26","subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Add error checking in\n\tgst_libcamera_src_task_enter()","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hey,\nI think I messed up with the commit history in this patch, do you want me\nto rebase the commit in v1 and v2 together and send a new patch ?\n\nRegards,\n*Vedant Paranjape*\n\nOn Wed, Jun 2, 2021 at 12:54 AM Vedant Paranjape <\nvedantparanjape160201@gmail.com> wrote:\n\n> Error checking the output from generateConfiguration() was missing in\n> the code. Only assert was added as a guard which checked if the size of\n> generated camera config was equal to size of roles passed to it.\n>\n> If the roles argument has a invalid member, it will return a nullptr and\n> then\n> trying to access a member on a nullptr for size comparison will result in a\n> segmentation fault. So, if the function returns a nullptr, it will simply\n> put\n> a error on GstBus and gracefully exit.\n>\n> Signed-off-by: Vedant Paranjape \n> ---\n> src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> b/src/gstreamer/gstlibcamerasrc.cpp\n> index 8b6057df..910c0f83 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -375,9 +375,9 @@ gst_libcamera_src_task_enter(GstTask *task,\n> [[maybe_unused]] GThread *thread,\n>\n> /* Generate the stream configurations, there should be one per\n> pad. */\n> state->config_ = state->cam_->generateConfiguration(roles);\n> - if (state->config_ == nullptr) {\n> + if (state->config_ == nullptr && state->config_->size() ==\n> state->srcpads_.size()) {\n> GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> - (\"Failed to generate camera\n> configuration from roles\"),\n> + (\"Failed to generate camera\n> configuration from\"),\n> (\"Camera::generateConfiguration()\n> returned nullptr\"));\n> gst_task_stop(task);\n> return;\n> --\n> 2.25.1\n>\n>","headers":{"Return-Path":"","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 B12E1C3206\n\tfor ;\n\tTue, 1 Jun 2021 19:27:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 335F46891F;\n\tTue, 1 Jun 2021 21:27:41 +0200 (CEST)","from mail-yb1-xb2f.google.com (mail-yb1-xb2f.google.com\n\t[IPv6:2607:f8b0:4864:20::b2f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A09936891F\n\tfor ;\n\tTue, 1 Jun 2021 21:27:39 +0200 (CEST)","by mail-yb1-xb2f.google.com with SMTP id i4so353912ybe.2\n\tfor ;\n\tTue, 01 Jun 2021 12:27:39 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"n/5DnoAK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to;\n\tbh=GljWzehH658xe5tsZ6iTWax68FnSpa7BgdeUMJbU7Uk=;\n\tb=n/5DnoAKZs031q8qUfEwZzjxjz1MYU5wTE/6PPfJucTYkFsa9W15rZikLe7e9wgQV0\n\t2VDyAMmuFhr8+S5JyWWsBhwOdHBhD3pXDHZck+56uNxA3Fozci2xb4eF104lVMcM85R6\n\tK+W386AnUzz5ApxV3KzPpiNyweSGzInfeT/UzsqSE8HwDKfkCDAXhLDkBISlJFYKkfGO\n\tDAHwULaZBeot9+y3CG70E6A+giFxQKf9jeY4SNNP/1bdbYrYyvcYIi2w1lVrlpXdt3mX\n\tLnP1wnC/zuKKg0CUXcK7z4kKdBs1/zR6r8MS+x1l9kakVZBvnMXrKcPjakF5H64SSfeq\n\tCbrA==","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;\n\tbh=GljWzehH658xe5tsZ6iTWax68FnSpa7BgdeUMJbU7Uk=;\n\tb=NQXIlj7A5OQLgpmnJg6LIyvVmUCJEJr3i+4DT2bcsj2KDDTU8bX4hWLQ1IR0Lg7ItZ\n\tosM97jS88rdZfpS5LMqQSeSqkStGnqU3pfLSYy8/v9fBw9ls+3kizejuKKh09D4/ps1D\n\teOKNlyJPFd0qwJY7GnF67TwIYHmBREMYuXxAr9i7SIRaF9TvJB54pFpRvq9u3hgxnNQ9\n\t6gwzJAq3QvIIGzBmOGSmSWvayAL0UQvOS6kPE7KSuO4Ua8pqf2NL0NxRuy0JXtw4DDzU\n\t5yTCt0p4vuO88yAN0vLebZgKA7KPic/8XfnCW1h10zGGpnq5NG1IeYhfsDghlWEbak5K\n\tUmHw==","X-Gm-Message-State":"AOAM533t1MX5gYR9b2nZPww9/ms1Uy68HpYU0TzuvuQ9jFPCsfNqc0im\n\tHb2KakAvOaM3vFc4ruiEg9HMMM8vQ8KU7GqM4mo0iTMQE2M=","X-Google-Smtp-Source":"ABdhPJy9l516jQd/uSEpFPDiCYaFFJZaceEUXYWGIKGFMz2EcXNBV1w8aOdKX90bnscS5mwWpdUVrbACDKuU9xSUedY=","X-Received":"by 2002:a5b:607:: with SMTP id d7mr19638836ybq.432.1622575658255;\n\tTue, 01 Jun 2021 12:27:38 -0700 (PDT)","MIME-Version":"1.0","References":"<20210601192405.143591-1-vedantparanjape160201@gmail.com>","In-Reply-To":"<20210601192405.143591-1-vedantparanjape160201@gmail.com>","From":"Vedant Paranjape ","Date":"Wed, 2 Jun 2021 00:57:26 +0530","Message-ID":"","To":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"000000000000bb47ac05c3b95555\"","Subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Add error checking in\n\tgst_libcamera_src_task_enter()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17364,"web_url":"https://patchwork.libcamera.org/comment/17364/","msgid":"","date":"2021-06-01T20:10:54","subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Add error checking in\n\tgst_libcamera_src_task_enter()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Vedant,\n\nOn Wed, Jun 02, 2021 at 12:57:26AM +0530, Vedant Paranjape wrote:\n> Hey,\n> I think I messed up with the commit history in this patch, do you want me\n> to rebase the commit in v1 and v2 together and send a new patch ?\n\nPlease do. As a general rule, patches should be based on the master\nbranch of libcamera. In this particular case, we won't apply v1 (given\nthat it didn't pass the review step), so v2 has to be self-contained.\nThere are multiple reasons for that, two major ones are to avoid\nbisection breakages (if v1 introduces a problem that gets noticed during\nreview, applying v1 and a fix on top of it would cause breakages when\ntesting the version with v1 without the fix), and to show the whole\ncontext during review (if I were to review this v2, I would need to\nfirst apply your v1, then v2, and review the result).\n\n> On Wed, Jun 2, 2021 at 12:54 AM Vedant Paranjape wrote:\n> > Error checking the output from generateConfiguration() was missing in\n> > the code. Only assert was added as a guard which checked if the size of\n> > generated camera config was equal to size of roles passed to it.\n> >\n> > If the roles argument has a invalid member, it will return a nullptr and then\n> > trying to access a member on a nullptr for size comparison will result in a\n> > segmentation fault. So, if the function returns a nullptr, it will simply put\n> > a error on GstBus and gracefully exit.\n> >\n> > Signed-off-by: Vedant Paranjape \n> > ---\n> > src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> > 1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> > b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 8b6057df..910c0f83 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -375,9 +375,9 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> >\n> > /* Generate the stream configurations, there should be one per pad. */\n> > state->config_ = state->cam_->generateConfiguration(roles);\n> > - if (state->config_ == nullptr) {\n> > + if (state->config_ == nullptr && state->config_->size() == state->srcpads_.size()) {\n> > GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > - (\"Failed to generate camera configuration from roles\"),\n> > + (\"Failed to generate camera configuration from\"),\n> > (\"Camera::generateConfiguration() returned nullptr\"));\n> > gst_task_stop(task);\n> > return;","headers":{"Return-Path":"","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 29C1DC3205\n\tfor ;\n\tTue, 1 Jun 2021 20:11:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A5C9A68920;\n\tTue, 1 Jun 2021 22:11:06 +0200 (CEST)","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 2AF566891F\n\tfor ;\n\tTue, 1 Jun 2021 22:11:05 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 982ABE78;\n\tTue, 1 Jun 2021 22:11:04 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"La3yA0WR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622578264;\n\tbh=FpXyhs7gM6OERCcp8rYr+CXdd4CKYIfYLfRxFX+dM6M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=La3yA0WRvKN9+mD4H29JDqDUx/0+KVdsunH62ao5lLYRkplFoFyWsiQnr+6KmFrhq\n\ttZKF9KigTIRhpb1OCBL6hDSvuHy/KrXn7F+qeMr+EZhHiqqmxHev1mHLM4Wc0jk1la\n\th73fM1we9kXpo1FGwqfwa0dTNC2/JpmYiEJM3evY=","Date":"Tue, 1 Jun 2021 23:10:54 +0300","From":"Laurent Pinchart ","To":"Vedant Paranjape ","Message-ID":"","References":"<20210601192405.143591-1-vedantparanjape160201@gmail.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Add error checking in\n\tgst_libcamera_src_task_enter()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17365,"web_url":"https://patchwork.libcamera.org/comment/17365/","msgid":"<8eeac05464b6e04863baf11ff0ec546fd6a51957.camel@ndufresne.ca>","date":"2021-06-01T20:13:49","subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Add error checking in\n\tgst_libcamera_src_task_enter()","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Hi Vedant,\n\nthanks for the patch.\n\nLe mercredi 02 juin 2021 à 00:54 +0530, Vedant Paranjape a écrit :\n> Error checking the output from generateConfiguration() was missing in\n> the code. Only assert was added as a guard which checked if the size of\n> generated camera config was equal to size of roles passed to it.\n> \n> If the roles argument has a invalid member, it will return a nullptr and then\n> trying to access a member on a nullptr for size comparison will result in a\n> segmentation fault. So, if the function returns a nullptr, it will simply put\n push\n> a error on GstBus and gracefully exit.\n an error message\n> \n> Signed-off-by: Vedant Paranjape \n> ---\n> src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 8b6057df..910c0f83 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -375,9 +375,9 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \n> \t/* Generate the stream configurations, there should be one per pad. */\n> \tstate->config_ = state->cam_->generateConfiguration(roles);\n> -\tif (state->config_ == nullptr) {\n> +\tif (state->config_ == nullptr && state->config_->size() == state->srcpads_.size()) {\n\nIf state->config_ is null, then the right part will be executed (true &&\nright_part) and will crash due to nullptr deference.\n\nI'd drop this patch, and resend v1 with the assert kept but placed after the\nnullptr handling branch.\n\n> \t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> -\t\t\t\t (\"Failed to generate camera configuration from roles\"),\n> +\t\t\t\t (\"Failed to generate camera configuration from\"),\n\nI agree the initial message was vague, but how the removing \"roles\" word helps ?\n\n> \t\t\t\t (\"Camera::generateConfiguration() returned nullptr\"));\n> \t\tgst_task_stop(task);\n> \t\treturn;","headers":{"Return-Path":"","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 B5B6EC3205\n\tfor ;\n\tTue, 1 Jun 2021 20:13:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3832D68925;\n\tTue, 1 Jun 2021 22:13:54 +0200 (CEST)","from mail-qk1-x734.google.com (mail-qk1-x734.google.com\n\t[IPv6:2607:f8b0:4864:20::734])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DCBBE6891F\n\tfor ;\n\tTue, 1 Jun 2021 22:13:51 +0200 (CEST)","by mail-qk1-x734.google.com with SMTP id j184so88033qkd.6\n\tfor ;\n\tTue, 01 Jun 2021 13:13:51 -0700 (PDT)","from nicolas-tpx395.localdomain (173-246-12-168.qc.cable.ebox.net.\n\t[173.246.12.168]) by smtp.gmail.com with ESMTPSA id\n\tc9sm12944292qke.8.2021.06.01.13.13.49\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 01 Jun 2021 13:13:50 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com\n\theader.b=\"NVZXaRY/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:date:in-reply-to:references:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=lY9OXO7XuBMjaBWOhE17xC++De/+2EYaZFDj1AlBMXg=;\n\tb=NVZXaRY/buxqYMdQDybOLoUfLhcvAuaxhT0Mak5iRzDgwylqdK2/q0k5yIx5RzpGFp\n\tLKuRIWmSBIBy7ldbQZLktrPb0vLAPnCNDoHRpHvWT43qWnV2QCTqSX1KJc+ECMuMAAfm\n\tnmhTieNkPnGl4PzzSbwtSplkNvwt5HVc/kqihVgJJpzLtjYLV71iP2creLB62JbLNoAO\n\t6ibiKzOXKxyV8nCpMp99qGIF63fvADnkn2VDaRQuNu2gAtBenhlyZ85PeAmtRJM1fCcW\n\t1+hU8BOtpijRixsqSTC4vU+Z2AokBRfPvHu1DaZs+pjD2ZtcRO93+5rmB7KGh/QZ3N6T\n\tF8tg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=lY9OXO7XuBMjaBWOhE17xC++De/+2EYaZFDj1AlBMXg=;\n\tb=B0mb4OZoqyGb868g0eiTN5/MvSihjGGMl213vLuXx3+Pse0q0JMD1mlY2XW937d31l\n\t0yzU6MWdWBUTJm/xvQ8Ksqolyb3bLOuOcoXmNGwDhj7/gO5xIGcX8U/2PltSsLtniAHz\n\tfG878Yt84GM75SVxBrVbWcgc/wyU5PBjzWOfIAZgi1DFRSV9Gbt2FU2Yt3zJOm/YGQJh\n\t6fPKFVWfbUPgLvngJzOcUbLSfYTGEdKti2WQYHPwHkFEf2s9KJwVxr4rxiXdKxr/Aqop\n\teXekU5vGr+TUtgnJqlj08LjoL0+pc34UMQCOliJNT8WEegCitL7zhrlUhz/PW3GYm2zU\n\tLD7Q==","X-Gm-Message-State":"AOAM533rdL8BdxBjA9VX/tlU1zvEmUnEoC3B3Tk/BeuJEqkf0U9ojcNr\n\tF0R+vrG4KX5BmpJKuwzZZEsJAw==","X-Google-Smtp-Source":"ABdhPJyR/29fv1rWygmRa+L0XtJR84WPt+zxN0Ylm5Gf2Z2IdFXV1PApr909a2Go75u6/sFQ3NIx3Q==","X-Received":"by 2002:a37:9003:: with SMTP id s3mr23415274qkd.86.1622578430628;\n\tTue, 01 Jun 2021 13:13:50 -0700 (PDT)","Message-ID":"<8eeac05464b6e04863baf11ff0ec546fd6a51957.camel@ndufresne.ca>","From":"Nicolas Dufresne ","To":"Vedant Paranjape , \n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 01 Jun 2021 16:13:49 -0400","In-Reply-To":"<20210601192405.143591-1-vedantparanjape160201@gmail.com>","References":"<20210601192405.143591-1-vedantparanjape160201@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.1 (3.40.1-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Add error checking in\n\tgst_libcamera_src_task_enter()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17366,"web_url":"https://patchwork.libcamera.org/comment/17366/","msgid":"","date":"2021-06-01T20:13:50","subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Add error checking in\n\tgst_libcamera_src_task_enter()","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hi,\nGot it. I'll send a completely new patch by merging the two commits on my\nlocal repo using git rebase.\n\nAlso, let me know if the commit message looks good to you. I did changes\naccording to your comments on the v1 patch\n\nRegards,\nVedant Paranjape\n\nOn Wed, 2 Jun, 2021, 01:41 Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Vedant,\n>\n> On Wed, Jun 02, 2021 at 12:57:26AM +0530, Vedant Paranjape wrote:\n> > Hey,\n> > I think I messed up with the commit history in this patch, do you want me\n> > to rebase the commit in v1 and v2 together and send a new patch ?\n>\n> Please do. As a general rule, patches should be based on the master\n> branch of libcamera. In this particular case, we won't apply v1 (given\n> that it didn't pass the review step), so v2 has to be self-contained.\n> There are multiple reasons for that, two major ones are to avoid\n> bisection breakages (if v1 introduces a problem that gets noticed during\n> review, applying v1 and a fix on top of it would cause breakages when\n> testing the version with v1 without the fix), and to show the whole\n> context during review (if I were to review this v2, I would need to\n> first apply your v1, then v2, and review the result).\n>\n> > On Wed, Jun 2, 2021 at 12:54 AM Vedant Paranjape wrote:\n> > > Error checking the output from generateConfiguration() was missing in\n> > > the code. Only assert was added as a guard which checked if the size of\n> > > generated camera config was equal to size of roles passed to it.\n> > >\n> > > If the roles argument has a invalid member, it will return a nullptr\n> and then\n> > > trying to access a member on a nullptr for size comparison will result\n> in a\n> > > segmentation fault. So, if the function returns a nullptr, it will\n> simply put\n> > > a error on GstBus and gracefully exit.\n> > >\n> > > Signed-off-by: Vedant Paranjape \n> > > ---\n> > > src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> > > 1 file changed, 2 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> > > b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index 8b6057df..910c0f83 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -375,9 +375,9 @@ gst_libcamera_src_task_enter(GstTask *task,\n> [[maybe_unused]] GThread *thread,\n> > >\n> > > /* Generate the stream configurations, there should be one per\n> pad. */\n> > > state->config_ = state->cam_->generateConfiguration(roles);\n> > > - if (state->config_ == nullptr) {\n> > > + if (state->config_ == nullptr && state->config_->size() ==\n> state->srcpads_.size()) {\n> > > GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > > - (\"Failed to generate camera\n> configuration from roles\"),\n> > > + (\"Failed to generate camera\n> configuration from\"),\n> > > (\"Camera::generateConfiguration()\n> returned nullptr\"));\n> > > gst_task_stop(task);\n> > > return;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 18780C3205\n\tfor ;\n\tTue, 1 Jun 2021 20:14:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D1BE068921;\n\tTue, 1 Jun 2021 22:14:03 +0200 (CEST)","from mail-yb1-xb34.google.com (mail-yb1-xb34.google.com\n\t[IPv6:2607:f8b0:4864:20::b34])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D96F26891F\n\tfor ;\n\tTue, 1 Jun 2021 22:14:02 +0200 (CEST)","by mail-yb1-xb34.google.com with SMTP id g38so455684ybi.12\n\tfor ;\n\tTue, 01 Jun 2021 13:14:02 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"i935SKl2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=fplqC8xIcF3yuQe/yC/vv+E1N5hL22UUAUUBNhkJgbc=;\n\tb=i935SKl2oRYg8NhDmb2rGunNcROAMJw46VDXqsTHptZvagAsNrUn5oOlxsOp203YmG\n\tzBUerSd0hWLW5TAb3mlDl99oo0BbCDG/A6rsB3c9kaftDNni4Wf58rppeK+2YbM+eFxu\n\tQyjCAJWWgXYvFBn40SZDiUy0HIppy+2tSHhNBhJJ+Kaovv0PIEkff0KfxkiWp2Qrx088\n\t4fOZBvsDpHquV1POmRT7I6fbD6JT3iYndko5kKmx1oDcH6A3BQCdc6aI1+haQJrVxFv9\n\tze0Fzcg12DMm+3FYcnUdfSuzH7fOwMDdZkVqHSbpTTbyFdqEZALhPj7kEm0uGj2x0XTd\n\tpLeQ==","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=fplqC8xIcF3yuQe/yC/vv+E1N5hL22UUAUUBNhkJgbc=;\n\tb=FHeVc1uSYuBAMVFQLPac5Anpysq/3bcFDNTzv00mVTf8myjw1CWj4vCXJJ7kv4dB/M\n\t7s9dnUHoPVN/WrqfiOY/ocdrXFUFp4RIJeIYe+TWZbfg8B23eN9TX+Fw0orSXISoRU2w\n\tgnZjrQp/y3XOtv0MiaV4rBjaJEFsDSE72fnj8jCxYy2v5ZbS9j3wb4TFcGq9hQ3e93SY\n\tsKd5TjZ7guQoiGWEcoFQJ1raE0Oe4qSoOXbs8sZ81lJD/TPAq7kRhriwC1QS55r13mxX\n\tcmE7FJG+Wdr/erV9HiLfLoqfFAPdhPCFwpH85HdZ7/bLjGn/2BlFuhzX30Ot2MJW9gWf\n\tCdOw==","X-Gm-Message-State":"AOAM531P0rlk8rPSV1G566/kGvHkepaTN2xT9bwsRJ1nEX86nWmTT/VZ\n\tX4EuGEQ/H4E7nterkZGoICKJDeyr6+w0Cr817vSozZ6/Pg8=","X-Google-Smtp-Source":"ABdhPJwGPjcH86bI6+u9bd3x2ridwE9eo6dp4MjbHYWIVZ/QSCXOjmbFr8fyxeVfv/kzXjUqZeKykHa1P1Z3n5xyZQs=","X-Received":"by 2002:a25:54a:: with SMTP id 71mr18502153ybf.223.1622578441543;\n\tTue, 01 Jun 2021 13:14:01 -0700 (PDT)","MIME-Version":"1.0","References":"<20210601192405.143591-1-vedantparanjape160201@gmail.com>\n\t\n\t","In-Reply-To":"","From":"Vedant Paranjape ","Date":"Wed, 2 Jun 2021 01:43:50 +0530","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000a0e28e05c3b9fbb7\"","Subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Add error checking in\n\tgst_libcamera_src_task_enter()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]