[{"id":1527,"web_url":"https://patchwork.libcamera.org/comment/1527/","msgid":"<20190428235600.eajzy4lsb5wdor52@uno.localdomain>","date":"2019-04-28T23:56:00","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Don't ignore the\n\treturn value of read() and write()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"HI Laurent,\n\nOn Mon, Apr 29, 2019 at 02:12:37AM +0300, Laurent Pinchart wrote:\n> The glibc read() and write() functions are defined with the\n> __warn_unused_result__ attribute when using FORTIFY_SOURCE. Don't ignore\n> their return value.\n>\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/event_dispatcher_poll.cpp |  8 ++++++--\n>  test/event.cpp                          | 13 +++++++++++--\n>  2 files changed, 17 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> index 1f0f352a8e0a..2621b7d96b1e 100644\n> --- a/src/libcamera/event_dispatcher_poll.cpp\n> +++ b/src/libcamera/event_dispatcher_poll.cpp\n> @@ -162,7 +162,9 @@ void EventDispatcherPoll::processEvents()\n>  void EventDispatcherPoll::interrupt()\n>  {\n>  \tuint64_t value = 1;\n> -\twrite(eventfd_, &value, sizeof(value));\n> +\tssize_t ret = write(eventfd_, &value, sizeof(value));\n> +\tif (ret < 0)\n> +\t\tLOG(Event, Error) << \"Failed to interrupt event dispatcher\";\n>  }\n>\n>  short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> @@ -214,7 +216,9 @@ void EventDispatcherPoll::processInterrupt(const struct pollfd &pfd)\n>  \t\treturn;\n>\n>  \tuint64_t value;\n> -\tread(eventfd_, &value, sizeof(value));\n> +\tssize_t ret = read(eventfd_, &value, sizeof(value));\n> +\tif (ret < 0)\n> +\t\tLOG(Event, Error) << \"Failed to process interrupt\";\n>  }\n>\n>  void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pollfds)\n> diff --git a/test/event.cpp b/test/event.cpp\n> index 52bc0c7e77f5..9bd876153a18 100644\n> --- a/test/event.cpp\n> +++ b/test/event.cpp\n> @@ -38,6 +38,7 @@ protected:\n>  \t\tEventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();\n>  \t\tstd::string data(\"H2G2\");\n>  \t\tTimer timeout;\n> +\t\tssize_t ret;\n>\n>  \t\tEventNotifier readNotifier(pipefd_[0], EventNotifier::Read);\n>  \t\treadNotifier.activated.connect(this, &EventTest::readReady);\n> @@ -46,7 +47,11 @@ protected:\n>  \t\tmemset(data_, 0, sizeof(data_));\n>  \t\tsize_ = 0;\n>\n> -\t\twrite(pipefd_[1], data.data(), data.size());\n> +\t\tret = write(pipefd_[1], data.data(), data.size());\n> +\t\tif (ret < 0) {\n> +\t\t\tcout << \"Pipe write failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n>\n>  \t\ttimeout.start(100);\n>  \t\tdispatcher->processEvents();\n> @@ -73,7 +78,11 @@ protected:\n>  \t\tnotified_ = false;\n>  \t\treadNotifier.setEnabled(false);\n>\n> -\t\twrite(pipefd_[1], data.data(), data.size());\n> +\t\tret = write(pipefd_[1], data.data(), data.size());\n> +\t\tif (ret < 0) {\n> +\t\t\tcout << \"Pipe write failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n>\n>  \t\ttimeout.start(100);\n>  \t\tdispatcher->processEvents();\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":"<jacopo@jmondi.org>","Received":["from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9F77F60E58\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Apr 2019 01:55:08 +0200 (CEST)","from uno.localdomain (net-37-182-44-227.cust.vodafonedsl.it\n\t[37.182.44.227]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 2EB1B20005;\n\tSun, 28 Apr 2019 23:55:07 +0000 (UTC)"],"X-Originating-IP":"37.182.44.227","Date":"Mon, 29 Apr 2019 01:56:00 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190428235600.eajzy4lsb5wdor52@uno.localdomain>","References":"<20190428231238.30547-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"u6sqmuncstjxs57t\"","Content-Disposition":"inline","In-Reply-To":"<20190428231238.30547-1-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Don't ignore the\n\treturn value of read() and write()","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, 28 Apr 2019 23:55:08 -0000"}},{"id":1530,"web_url":"https://patchwork.libcamera.org/comment/1530/","msgid":"<1240def7-e379-500a-8019-93ffd73c0684@ideasonboard.com>","date":"2019-04-29T12:13:31","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Don't ignore the\n\treturn value of read() and write()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 29/04/2019 00:12, Laurent Pinchart wrote:\n> The glibc read() and write() functions are defined with the\n> __warn_unused_result__ attribute when using FORTIFY_SOURCE. Don't ignore\n> their return value.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/event_dispatcher_poll.cpp |  8 ++++++--\n>  test/event.cpp                          | 13 +++++++++++--\n>  2 files changed, 17 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> index 1f0f352a8e0a..2621b7d96b1e 100644\n> --- a/src/libcamera/event_dispatcher_poll.cpp\n> +++ b/src/libcamera/event_dispatcher_poll.cpp\n> @@ -162,7 +162,9 @@ void EventDispatcherPoll::processEvents()\n>  void EventDispatcherPoll::interrupt()\n>  {\n>  \tuint64_t value = 1;\n> -\twrite(eventfd_, &value, sizeof(value));\n> +\tssize_t ret = write(eventfd_, &value, sizeof(value));> +\tif (ret < 0)\n\nDo we care if ret != sizeof(value)? (I think it must be highly unlikely\nthat the read/write call doesn't at least read the 64 bit value...)\n\n> +\t\tLOG(Event, Error) << \"Failed to interrupt event dispatcher\";\n\nWhat about printing the strerror(errno), as if this ever actually\nhappens (even if unlikely) it would be useful...\n\n>  }\n>  \n>  short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> @@ -214,7 +216,9 @@ void EventDispatcherPoll::processInterrupt(const struct pollfd &pfd)\n>  \t\treturn;\n>  \n>  \tuint64_t value;\n> -\tread(eventfd_, &value, sizeof(value));\n> +\tssize_t ret = read(eventfd_, &value, sizeof(value));\n> +\tif (ret < 0)\n> +\t\tLOG(Event, Error) << \"Failed to process interrupt\";\n\n\nSame comments as above...\n\nCould we ever expect ret == 0 on the read call (EOF?)? I presume as\nwe're checking with poll() in advance this should always have at least\none event to read.\n\n\nI think the return values are probably fine, and the errno is optional\nso with or without updates:\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n>  }\n>  \n>  void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pollfds)\n> diff --git a/test/event.cpp b/test/event.cpp\n> index 52bc0c7e77f5..9bd876153a18 100644\n> --- a/test/event.cpp\n> +++ b/test/event.cpp\n> @@ -38,6 +38,7 @@ protected:\n>  \t\tEventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();\n>  \t\tstd::string data(\"H2G2\");\n>  \t\tTimer timeout;\n> +\t\tssize_t ret;\n>  \n>  \t\tEventNotifier readNotifier(pipefd_[0], EventNotifier::Read);\n>  \t\treadNotifier.activated.connect(this, &EventTest::readReady);\n> @@ -46,7 +47,11 @@ protected:\n>  \t\tmemset(data_, 0, sizeof(data_));\n>  \t\tsize_ = 0;\n>  \n> -\t\twrite(pipefd_[1], data.data(), data.size());\n> +\t\tret = write(pipefd_[1], data.data(), data.size());\n> +\t\tif (ret < 0) {\n> +\t\t\tcout << \"Pipe write failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n>  \n>  \t\ttimeout.start(100);\n>  \t\tdispatcher->processEvents();\n> @@ -73,7 +78,11 @@ protected:\n>  \t\tnotified_ = false;\n>  \t\treadNotifier.setEnabled(false);\n>  \n> -\t\twrite(pipefd_[1], data.data(), data.size());\n> +\t\tret = write(pipefd_[1], data.data(), data.size());\n> +\t\tif (ret < 0) {\n> +\t\t\tcout << \"Pipe write failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n>  \n>  \t\ttimeout.start(100);\n>  \t\tdispatcher->processEvents();\n>","headers":{"Return-Path":"<kieran.bingham@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 E7E3F60C46\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Apr 2019 14:13:34 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 442122E7;\n\tMon, 29 Apr 2019 14:13:34 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1556540014;\n\tbh=MGi8E4ZDp8KQ6FQqaoJJroO2ewlEmXYlq2tOAhRwU4c=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=M9lIGrPj2JWTqGtKFTZG5L9Ef/GRHC9FYTaDNyBHjjAY1n5OAdseOH0jl/Hz3ezv7\n\tIvkKuOJEaHCz0oI9jIIGfeapOU21HGAA2ynMm1WIE8SxKj+pNr0InL1QP43St+LtM/\n\tYt653v+QjJSuLX3L939j1dxhfPnpHKEQg4QWucM4=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190428231238.30547-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<1240def7-e379-500a-8019-93ffd73c0684@ideasonboard.com>","Date":"Mon, 29 Apr 2019 13:13:31 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.6.1","MIME-Version":"1.0","In-Reply-To":"<20190428231238.30547-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Don't ignore the\n\treturn value of read() and write()","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":"Mon, 29 Apr 2019 12:13:35 -0000"}},{"id":1531,"web_url":"https://patchwork.libcamera.org/comment/1531/","msgid":"<20190429122554.GB24837@pendragon.ideasonboard.com>","date":"2019-04-29T12:25:54","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Don't ignore the\n\treturn value of read() and write()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Apr 29, 2019 at 01:13:31PM +0100, Kieran Bingham wrote:\n> On 29/04/2019 00:12, Laurent Pinchart wrote:\n> > The glibc read() and write() functions are defined with the\n> > __warn_unused_result__ attribute when using FORTIFY_SOURCE. Don't ignore\n> > their return value.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/libcamera/event_dispatcher_poll.cpp |  8 ++++++--\n> >  test/event.cpp                          | 13 +++++++++++--\n> >  2 files changed, 17 insertions(+), 4 deletions(-)\n> > \n> > diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> > index 1f0f352a8e0a..2621b7d96b1e 100644\n> > --- a/src/libcamera/event_dispatcher_poll.cpp\n> > +++ b/src/libcamera/event_dispatcher_poll.cpp\n> > @@ -162,7 +162,9 @@ void EventDispatcherPoll::processEvents()\n> >  void EventDispatcherPoll::interrupt()\n> >  {\n> >  \tuint64_t value = 1;\n> > -\twrite(eventfd_, &value, sizeof(value));\n> > +\tssize_t ret = write(eventfd_, &value, sizeof(value));> +\tif (ret < 0)\n> \n> Do we care if ret != sizeof(value)? (I think it must be highly unlikely\n> that the read/write call doesn't at least read the 64 bit value...)\n> \n> > +\t\tLOG(Event, Error) << \"Failed to interrupt event dispatcher\";\n> \n> What about printing the strerror(errno), as if this ever actually\n> happens (even if unlikely) it would be useful...\n\nGood point. I'll fix this.\n\n> >  }\n> >  \n> >  short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> > @@ -214,7 +216,9 @@ void EventDispatcherPoll::processInterrupt(const struct pollfd &pfd)\n> >  \t\treturn;\n> >  \n> >  \tuint64_t value;\n> > -\tread(eventfd_, &value, sizeof(value));\n> > +\tssize_t ret = read(eventfd_, &value, sizeof(value));\n> > +\tif (ret < 0)\n> > +\t\tLOG(Event, Error) << \"Failed to process interrupt\";\n> \n> Same comments as above...\n> \n> Could we ever expect ret == 0 on the read call (EOF?)? I presume as\n> we're checking with poll() in advance this should always have at least\n> one event to read.\n\nYes, there should also be data to read.\n\n> I think the return values are probably fine, and the errno is optional\n> so with or without updates:\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> >  }\n> >  \n> >  void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pollfds)\n> > diff --git a/test/event.cpp b/test/event.cpp\n> > index 52bc0c7e77f5..9bd876153a18 100644\n> > --- a/test/event.cpp\n> > +++ b/test/event.cpp\n> > @@ -38,6 +38,7 @@ protected:\n> >  \t\tEventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();\n> >  \t\tstd::string data(\"H2G2\");\n> >  \t\tTimer timeout;\n> > +\t\tssize_t ret;\n> >  \n> >  \t\tEventNotifier readNotifier(pipefd_[0], EventNotifier::Read);\n> >  \t\treadNotifier.activated.connect(this, &EventTest::readReady);\n> > @@ -46,7 +47,11 @@ protected:\n> >  \t\tmemset(data_, 0, sizeof(data_));\n> >  \t\tsize_ = 0;\n> >  \n> > -\t\twrite(pipefd_[1], data.data(), data.size());\n> > +\t\tret = write(pipefd_[1], data.data(), data.size());\n> > +\t\tif (ret < 0) {\n> > +\t\t\tcout << \"Pipe write failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> >  \n> >  \t\ttimeout.start(100);\n> >  \t\tdispatcher->processEvents();\n> > @@ -73,7 +78,11 @@ protected:\n> >  \t\tnotified_ = false;\n> >  \t\treadNotifier.setEnabled(false);\n> >  \n> > -\t\twrite(pipefd_[1], data.data(), data.size());\n> > +\t\tret = write(pipefd_[1], data.data(), data.size());\n> > +\t\tif (ret < 0) {\n> > +\t\t\tcout << \"Pipe write failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> >  \n> >  \t\ttimeout.start(100);\n> >  \t\tdispatcher->processEvents();","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C846060C46\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Apr 2019 14:26:06 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(net-37-182-44-227.cust.vodafonedsl.it [37.182.44.227])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 41D0C2E7;\n\tMon, 29 Apr 2019 14:26:06 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1556540766;\n\tbh=izrtcTbibpBICVqW/uIgi9LIngqJLQRao7m/EPKBIgM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Wh7yIEApnnxWWzdSCXtsMW2STCk0tB4uaAKUL1mMPtaYN/NwGojrWfjyovH42UQJm\n\tLnJubZUNnku4Nf1BiAkSVXcFzpTLytI7QTa1BGerA+shHQuPBKEKJcsB34DgEWPcfa\n\tD42zZendqI7n+C4q5T8qUXipc8kdxUo5TXAEpZy0=","Date":"Mon, 29 Apr 2019 15:25:54 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190429122554.GB24837@pendragon.ideasonboard.com>","References":"<20190428231238.30547-1-laurent.pinchart@ideasonboard.com>\n\t<1240def7-e379-500a-8019-93ffd73c0684@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<1240def7-e379-500a-8019-93ffd73c0684@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Don't ignore the\n\treturn value of read() and write()","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":"Mon, 29 Apr 2019 12:26:07 -0000"}}]