[{"id":61,"web_url":"https://patchwork.libcamera.org/comment/61/","msgid":"<2064136.iSg810hWVZ@avalon>","date":"2018-12-21T04:42:20","subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Friday, 21 December 2018 02:53:29 EET Niklas Söderlund wrote:\n> Calling the cleanup() function in the base class Test destructor only\n> calls the base class empty cleanup() function, not the overloaded one.\n> This results in tests not cleaning up after themself. Solve this by\n> explicitly calling the cleanup() function from execute().\n> \n> This was discovered while running valgrind on tests where objects where\n> allocated in init() and freed in cleanup().\n\nMy bad :-/\n\nDestructors must never call virtual methods, as during the execution of a \ndestructor the object behaves as if its dynamic type was identical to the \nstatic type of the destructor. The reason is simple : destructors are called \nin sequence from the leaf class to the root class. The Test destructor thus \ncan access any member of a derive class, as it will already have been \ndestructed.\n\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  test/test.cpp | 7 +++++--\n>  1 file changed, 5 insertions(+), 2 deletions(-)\n> \n> diff --git a/test/test.cpp b/test/test.cpp\n> index 4e7779e750d56687..1bb6ebcb9e8acf18 100644\n> --- a/test/test.cpp\n> +++ b/test/test.cpp\n> @@ -13,7 +13,6 @@ Test::Test()\n> \n>  Test::~Test()\n>  {\n> -\tcleanup();\n>  }\n> \n>  int Test::execute()\n> @@ -24,5 +23,9 @@ int Test::execute()\n>  \tif (ret < 0)\n>  \t\treturn ret;\n> \n> -\treturn run();\n> +\tret = run();\n> +\n> +\tcleanup();\n> +\n> +\treturn ret;\n>  }","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 41A0860B21\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Dec 2018 05:41:25 +0100 (CET)","from avalon.localnet (unknown [164.5.176.2])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C998C558;\n\tFri, 21 Dec 2018 05:41:24 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1545367284;\n\tbh=V0ZCux2EYrbaxPhMPrE52OpnrcZ8XdcExcCeQO7d83s=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=GmFZfLfxvB2SHMKEjnV2+Vqg068VYsOKzrZtcYAZYBPWuYXw9iKKsoZUOYyqfK4TL\n\thtTqoVhWdde+TCzZU8d0O8qNan1pm8wE6uxlZ4eXCUeigdULNEVli4BIWX8pcLGguq\n\tWtYKevQ4u4njseUlDdyhNb+M3d2aWmahYWkSqo/U=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 21 Dec 2018 06:42:20 +0200","Message-ID":"<2064136.iSg810hWVZ@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20181221005329.13597-1-niklas.soderlund@ragnatech.se>","References":"<20181221005329.13597-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"","Subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Fri, 21 Dec 2018 04:41:25 -0000"}},{"id":71,"web_url":"https://patchwork.libcamera.org/comment/71/","msgid":"<20181221162642.GO5597@w540>","date":"2018-12-21T16:26:42","subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Dec 21, 2018 at 01:53:29AM +0100, Niklas Söderlund wrote:\n> Calling the cleanup() function in the base class Test destructor only\n> calls the base class empty cleanup() function, not the overloaded one.\n> This results in tests not cleaning up after themself. Solve this by\n> explicitly calling the cleanup() function from execute().\n>\n> This was discovered while running valgrind on tests where objects where\n> allocated in init() and freed in cleanup().\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  test/test.cpp | 7 +++++--\n>  1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/test/test.cpp b/test/test.cpp\n> index 4e7779e750d56687..1bb6ebcb9e8acf18 100644\n> --- a/test/test.cpp\n> +++ b/test/test.cpp\n> @@ -13,7 +13,6 @@ Test::Test()\n>\n>  Test::~Test()\n>  {\n> -\tcleanup();\n>  }\n>\n>  int Test::execute()\n> @@ -24,5 +23,9 @@ int Test::execute()\n>  \tif (ret < 0)\n\nShouldn't the same be done here?\n\n>  \t\treturn ret;\n>\n> -\treturn run();\n> +\tret = run();\n> +\n> +\tcleanup();\n> +\n> +\treturn ret;\n>  }\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2321760B0B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Dec 2018 17:26:44 +0100 (CET)","from w540 (2-224-242-101.ip172.fastwebnet.it [2.224.242.101])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id B60EB4000F;\n\tFri, 21 Dec 2018 16:26:43 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 21 Dec 2018 17:26:42 +0100","From":"jacopo mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20181221162642.GO5597@w540>","References":"<20181221005329.13597-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"QQ8SllDnVL7XJJuW\"","Content-Disposition":"inline","In-Reply-To":"<20181221005329.13597-1-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.5.24 (2015-08-30)","Subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Fri, 21 Dec 2018 16:26:44 -0000"}},{"id":72,"web_url":"https://patchwork.libcamera.org/comment/72/","msgid":"<1902717.xIEUeLuhrf@avalon>","date":"2018-12-22T10:34:22","subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Friday, 21 December 2018 18:26:42 EET jacopo mondi wrote:\n> On Fri, Dec 21, 2018 at 01:53:29AM +0100, Niklas Söderlund wrote:\n> > Calling the cleanup() function in the base class Test destructor only\n> > calls the base class empty cleanup() function, not the overloaded one.\n> > This results in tests not cleaning up after themself. Solve this by\n> > explicitly calling the cleanup() function from execute().\n> > \n> > This was discovered while running valgrind on tests where objects where\n> > allocated in init() and freed in cleanup().\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> > \n> >  test/test.cpp | 7 +++++--\n> >  1 file changed, 5 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/test/test.cpp b/test/test.cpp\n> > index 4e7779e750d56687..1bb6ebcb9e8acf18 100644\n> > --- a/test/test.cpp\n> > +++ b/test/test.cpp\n> > @@ -13,7 +13,6 @@ Test::Test()\n> >  Test::~Test()\n> >  {\n> > -\tcleanup();\n> >  }\n> >  \n> >  int Test::execute()\n> > @@ -24,5 +23,9 @@ int Test::execute()\n> >  \tif (ret < 0)\n> \n> Shouldn't the same be done here?\n\nIt's a good question. Shouldn't the init() method clean after itself if it \nfails ? What's your opinion on this generally ?\n\n> >  \t\treturn ret;\n> > \n> > -\treturn run();\n> > +\tret = run();\n> > +\n> > +\tcleanup();\n> > +\n> > +\treturn ret;\n> >  }","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 82B4E60B21\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 22 Dec 2018 11:33:31 +0100 (CET)","from avalon.localnet (42-16-62-37.mobileinternet.proximus.be\n\t[37.62.16.42])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 70B594F8;\n\tSat, 22 Dec 2018 11:33:30 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1545474811;\n\tbh=QCx73gpX/TsmM1FDFQa4M2/TEjxvl0pmMOFCGFrgdTI=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=XTT3lJ0AaTUruICglZYqIljFibU6qVzIVic8sYAh4ukUnEf5CjPvRkEBqRordh9t2\n\tNo2iM7Cm/l6Ii1hSE9tHn9TCw/eVvsgGuyCWaIGWDuXkkNl25JyF97m1hO7DnkjOlA\n\tvYJ3Q7bnXaBTsWW0VWehOuV1AmpnOtKHACL7d4vY=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Sat, 22 Dec 2018 12:34:22 +0200","Message-ID":"<1902717.xIEUeLuhrf@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20181221162642.GO5597@w540>","References":"<20181221005329.13597-1-niklas.soderlund@ragnatech.se>\n\t<20181221162642.GO5597@w540>","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"","Subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Sat, 22 Dec 2018 10:33:31 -0000"}},{"id":73,"web_url":"https://patchwork.libcamera.org/comment/73/","msgid":"<20181222144410.GB19796@bigcity.dyn.berto.se>","date":"2018-12-22T14:44:10","subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo, Laurent,\n\nOn 2018-12-22 12:34:22 +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> On Friday, 21 December 2018 18:26:42 EET jacopo mondi wrote:\n> > On Fri, Dec 21, 2018 at 01:53:29AM +0100, Niklas Söderlund wrote:\n> > > Calling the cleanup() function in the base class Test destructor only\n> > > calls the base class empty cleanup() function, not the overloaded one.\n> > > This results in tests not cleaning up after themself. Solve this by\n> > > explicitly calling the cleanup() function from execute().\n> > > \n> > > This was discovered while running valgrind on tests where objects where\n> > > allocated in init() and freed in cleanup().\n> > > \n> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > ---\n> > > \n> > >  test/test.cpp | 7 +++++--\n> > >  1 file changed, 5 insertions(+), 2 deletions(-)\n> > > \n> > > diff --git a/test/test.cpp b/test/test.cpp\n> > > index 4e7779e750d56687..1bb6ebcb9e8acf18 100644\n> > > --- a/test/test.cpp\n> > > +++ b/test/test.cpp\n> > > @@ -13,7 +13,6 @@ Test::Test()\n> > >  Test::~Test()\n> > >  {\n> > > -\tcleanup();\n> > >  }\n> > >  \n> > >  int Test::execute()\n> > > @@ -24,5 +23,9 @@ int Test::execute()\n> > >  \tif (ret < 0)\n> > \n> > Shouldn't the same be done here?\n> \n> It's a good question. Shouldn't the init() method clean after itself if it \n> fails ? What's your opinion on this generally ?\n\nMy view is that in a perfect world cleanup() should be called here and \neach test cases implementation of cleanup() should be prepared to handle \npartially initialized state. Even better in a super perfect world init() \nand run() would never fail so there would be no need to check the return \ncodes or write the test to begin with as it would never fail anyhow ;-P\n\nAs we live in an in-perfect world I see little point in calling or not \ncalling cleanup() here, just bail out and make sure it's clear the test \nfailed.  If the state created by a half run init()/cealnup() function \ncrashes my machine it's a inconvenience but it would probably motivate \nme to find and fix the issue quicker :-)\n\n> \n> > >  \t\treturn ret;\n> > > \n> > > -\treturn run();\n> > > +\tret = run();\n> > > +\n> > > +\tcleanup();\n> > > +\n> > > +\treturn ret;\n> > >  }\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> \n>","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0F87E60B0C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 22 Dec 2018 15:44:13 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id c19-v6so7203913lja.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 22 Dec 2018 06:44:12 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tq10-v6sm5855190ljh.72.2018.12.22.06.44.10\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSat, 22 Dec 2018 06:44:11 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=KzXyGd4HqMu7wzxOdnktxjI9cAwzKlSzpxV6+2PWA/c=;\n\tb=JezZkj+B2RaCWc7vHuo93WspedDra+Iic08prG/ZPjJvmJQsWs7E1NneMejtffazC4\n\t4cD5VqAaWWF+pR5pT4vWcKMhkp6Az9xCRxnNx495WeodMlAtiepAvoIZgTHKkV5XJby5\n\tHDDGQTsJsPXiOStqSAHHjxo6ef32xFuKfFuel76yA/1zhxooU2Of8/e2O2BeNnl+BSxx\n\tMbwQO03LntYI3HpF+alkK6gSF+NT5r7/yoNQpYEw9IM8SVsl7NKCA6RFvQ+pW8UT6Jlc\n\tbf00Siesh0rOvWzENOVA9A8/Iyx9CAOdZy7YTFoXrWpMbXX26R8l4SCkxqBZLGgN4/BT\n\tr6Eg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=KzXyGd4HqMu7wzxOdnktxjI9cAwzKlSzpxV6+2PWA/c=;\n\tb=oaUuAe/nrXFCXr24QCN+FGBj8mTsed7RlCpv0t7wtHJfkfICaJao9g1Bb9jWMysJ25\n\t1t3K8XHdybFxsevavbPbzHMYjyoZkKBSxLctF+MaxyK3FBDu5+TKLx//gKUJBRZOkR1R\n\tD/x9to/dFhmckhNdiaud7MKk3U4ZRU/bUD8pg7A9h5wjAorHgm4dKsm4wM7fvAKWsPmF\n\taSyNWxrttuGvLDWHRwE2Zk/vC6eZEIdIaTJ8ry9MWeN+orbpRfMIAtdmj9dsXegN0K+q\n\tcf0/XnRT0OPRmeLYa5hKtj0fM8d8oBAkw8OT8WKOFA5COfw311gKwB4R8C0LzqY53MBm\n\trtyQ==","X-Gm-Message-State":"AJcUukfEmOdUbFkIEvOdB9yrc/oWxbQuqTTxNEuZBxP+zHLJnOza8NWN\n\tQUrcZ0h+Y5fhoFlX15L3VvJiKg==","X-Google-Smtp-Source":"ALg8bN7Y1636VOMbqZPguBL7XPcsVqF35mRaNLjaTaXyX51+zyFfV6/KaxFGWqvgf0ZCflK5/WDzuw==","X-Received":"by 2002:a2e:6f11:: with SMTP id\n\tk17-v6mr3941989ljc.94.1545489852103; \n\tSat, 22 Dec 2018 06:44:12 -0800 (PST)","Date":"Sat, 22 Dec 2018 15:44:10 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, jacopo mondi <jacopo@jmondi.org>","Message-ID":"<20181222144410.GB19796@bigcity.dyn.berto.se>","References":"<20181221005329.13597-1-niklas.soderlund@ragnatech.se>\n\t<20181221162642.GO5597@w540> <1902717.xIEUeLuhrf@avalon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<1902717.xIEUeLuhrf@avalon>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Sat, 22 Dec 2018 14:44:13 -0000"}},{"id":76,"web_url":"https://patchwork.libcamera.org/comment/76/","msgid":"<20181223210404.GA16050@archlinux>","date":"2018-12-23T21:04:04","subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent, Niklas,\n\nOn Sat, Dec 22, 2018 at 03:44:10PM +0100, Niklas S?derlund wrote:\n> Hi Jacopo, Laurent,\n>\n> On 2018-12-22 12:34:22 +0200, Laurent Pinchart wrote:\n> > Hi Jacopo,\n> >\n> > On Friday, 21 December 2018 18:26:42 EET jacopo mondi wrote:\n> > > On Fri, Dec 21, 2018 at 01:53:29AM +0100, Niklas Söderlund wrote:\n> > > > Calling the cleanup() function in the base class Test destructor only\n> > > > calls the base class empty cleanup() function, not the overloaded one.\n> > > > This results in tests not cleaning up after themself. Solve this by\n> > > > explicitly calling the cleanup() function from execute().\n> > > >\n> > > > This was discovered while running valgrind on tests where objects where\n> > > > allocated in init() and freed in cleanup().\n> > > >\n> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > > ---\n> > > >\n> > > >  test/test.cpp | 7 +++++--\n> > > >  1 file changed, 5 insertions(+), 2 deletions(-)\n> > > >\n> > > > diff --git a/test/test.cpp b/test/test.cpp\n> > > > index 4e7779e750d56687..1bb6ebcb9e8acf18 100644\n> > > > --- a/test/test.cpp\n> > > > +++ b/test/test.cpp\n> > > > @@ -13,7 +13,6 @@ Test::Test()\n> > > >  Test::~Test()\n> > > >  {\n> > > > -\tcleanup();\n> > > >  }\n> > > >\n> > > >  int Test::execute()\n> > > > @@ -24,5 +23,9 @@ int Test::execute()\n> > > >  \tif (ret < 0)\n> > >\n> > > Shouldn't the same be done here?\n> >\n> > It's a good question. Shouldn't the init() method clean after itself if it\n> > fails ? What's your opinion on this generally ?\n>\n> My view is that in a perfect world cleanup() should be called here and\n> each test cases implementation of cleanup() should be prepared to handle\n> partially initialized state. Even better in a super perfect world init()\n> and run() would never fail so there would be no need to check the return\n> codes or write the test to begin with as it would never fail anyhow ;-P\n>\n> As we live in an in-perfect world I see little point in calling or not\n> calling cleanup() here, just bail out and make sure it's clear the test\n> failed.  If the state created by a half run init()/cealnup() function\n> crashes my machine it's a inconvenience but it would probably motivate\n> me to find and fix the issue quicker :-)\n>\n\nI see, but as much as I would like to have init() to cleanup after\nitself, that should might not happen.\n\nWhat if the base class provided macro TEST_REGISTER does that in case\nof both success and failures?\n\n#define TEST_REGISTER(klass)\t\t\t\t\t\t\\\nint main(int argc, char *argv[])\t\t\t\t\t\\\n{\t\t\t\t\t\t\t\t\t\\\n\tint ret = klass().execute();\t\t\t\t\t\\\n        cleanup();                                                      \\\n                                                                        \\\n        return ret;                                                     \\\n}\n\nThanks\n   j\n\n\n> >\n> > > >  \t\treturn ret;\n> > > >\n> > > > -\treturn run();\n> > > > +\tret = run();\n> > > > +\n> > > > +\tcleanup();\n> > > > +\n> > > > +\treturn ret;\n> > > >  }\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n> >\n> >\n> >\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87A4B60B21\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 23 Dec 2018 22:04:14 +0100 (CET)","from archlinux (host54-51-dynamic.16-87-r.retail.telecomitalia.it\n\t[87.16.51.54]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id A022760003;\n\tSun, 23 Dec 2018 21:04:13 +0000 (UTC)"],"X-Originating-IP":"87.16.51.54","Date":"Sun, 23 Dec 2018 22:04:04 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas S?derlund <niklas.soderlund@ragnatech.se>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20181223210404.GA16050@archlinux>","References":"<20181221005329.13597-1-niklas.soderlund@ragnatech.se>\n\t<20181221162642.GO5597@w540> <1902717.xIEUeLuhrf@avalon>\n\t<20181222144410.GB19796@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"/04w6evG8XlLl3ft\"","Content-Disposition":"inline","In-Reply-To":"<20181222144410.GB19796@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.11.1 (2018-12-01)","Subject":"Re: [libcamera-devel] [PATCH] tests: call the derived Test class\n\tcleanup() function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Sun, 23 Dec 2018 21:04:14 -0000"}}]