[{"id":3270,"web_url":"https://patchwork.libcamera.org/comment/3270/","msgid":"<20191217021032.GP4856@pendragon.ideasonboard.com>","date":"2019-12-17T02:10:32","subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: file_descriptor: Add test","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 Tue, Dec 17, 2019 at 02:51:06AM +0100, Niklas Söderlund wrote:\n> Add a test which exercises the whole FileDescriptor interface.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  test/file_descriptor.cpp | 165 +++++++++++++++++++++++++++++++++++++++\n>  test/meson.build         |   1 +\n>  2 files changed, 166 insertions(+)\n>  create mode 100644 test/file_descriptor.cpp\n> \n> diff --git a/test/file_descriptor.cpp b/test/file_descriptor.cpp\n> new file mode 100644\n> index 0000000000000000..177d8bcf16591185\n> --- /dev/null\n> +++ b/test/file_descriptor.cpp\n> @@ -0,0 +1,165 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * file_descriptor.cpp - FileDescriptor test\n> + */\n> +\n> +#include <fcntl.h>\n> +#include <sys/stat.h>\n> +#include <sys/types.h>\n> +#include <unistd.h>\n> +\n> +#include <libcamera/file_descriptor.h>\n> +\n> +#include \"test.h\"\n> +\n> +using namespace libcamera;\n> +using namespace std;\n> +\n> +class FileDescriptorTest : public Test\n> +{\n> +protected:\n> +\tint init()\n> +\t{\n> +\t\tdesc1_ = nullptr;\n> +\t\tdesc2_ = nullptr;\n> +\n> +\t\tfd_ = open(\"/tmp\", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);\n> +\t\tif (fd_ < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Cache inode number of temp file. */\n> +\t\tstruct stat s;\n> +\t\tif (fstat(fd_, &s))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tinodeNr_ = s.st_ino;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\t/* Test creating FileDescriptor from numerical file descriptor. */\n> +\t\tdesc1_ = new FileDescriptor(fd_);\n> +\t\tif (desc1_->fd() == fd_)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!isValidFd(fd_) || !isValidFd(desc1_->fd()))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tint fd = desc1_->fd();\n> +\n> +\t\tdelete desc1_;\n> +\t\tdesc1_ = nullptr;\n> +\n> +\t\tif (!isValidFd(fd_) || isValidFd(fd))\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Test creating FileDescriptor from other FileDescriptor. */\n> +\t\tdesc1_ = new FileDescriptor(fd_);\n> +\t\tdesc2_ = new FileDescriptor(*desc1_);\n> +\n> +\t\tif (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() == desc2_->fd())\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tdelete desc1_;\n> +\t\tdesc1_ = nullptr;\n> +\n> +\t\tif (!isValidFd(desc2_->fd()))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tdelete desc2_;\n> +\t\tdesc2_ = nullptr;\n> +\n> +\t\t/* Test creating FileDescriptor by taking over other FileDescriptor. */\n> +\t\tdesc1_ = new FileDescriptor(fd_);\n> +\t\tfd = desc1_->fd();\n> +\t\tdesc2_ = new FileDescriptor(std::move(*desc1_));\n> +\n> +\t\tif (desc1_->fd() != -1 || desc2_->fd() != fd)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!isValidFd(desc2_->fd()))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tdelete desc1_;\n> +\t\tdesc1_ = nullptr;\n> +\t\tdelete desc2_;\n> +\t\tdesc2_ = nullptr;\n> +\n> +\t\t/* Test creating FileDescriptor by copy assignment. */\n> +\t\tdesc1_ = new FileDescriptor();\n> +\t\tdesc2_ = new FileDescriptor(fd_);\n> +\n> +\t\tif (desc1_->fd() != -1)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tfd = desc2_->fd();\n> +\t\t*desc1_ = *desc2_;\n> +\n> +\t\tif (desc1_->fd() == fd || desc2_->fd() != fd)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tdelete desc1_;\n> +\t\tdesc1_ = nullptr;\n> +\t\tdelete desc2_;\n> +\t\tdesc2_ = nullptr;\n> +\n> +\t\tif (!isValidFd(fd_))\n> +\t\t\treturn TestFail;\n\nYou can drop this check.\n\n> +\n> +\t\t/* Test creating FileDescriptor by move assignment. */\n> +\t\tdesc1_ = new FileDescriptor();\n> +\t\tdesc2_ = new FileDescriptor(fd_);\n> +\n> +\t\tfd = desc2_->fd();\n> +\t\t*desc1_ = std::move(*desc2_);\n> +\n> +\t\tif (desc1_->fd() != fd || desc2_->fd() != -1)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!isValidFd(desc1_->fd()))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tdelete desc1_;\n> +\t\tdesc1_ = nullptr;\n> +\t\tdelete desc2_;\n> +\t\tdesc2_ = nullptr;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tvoid cleanup()\n> +\t{\n> +\t\tdelete desc2_;\n> +\t\tdelete desc1_;\n> +\n> +\t\tif (fd_ > 0)\n> +\t\t\tclose(fd_);\n> +\t}\n> +\n> +private:\n> +\tbool isValidFd(int fd)\n> +\t{\n> +\t\tstruct stat s;\n> +\t\tif (fstat(fd, &s))\n> +\t\t\treturn false;\n> +\n> +\t\t/* Check that inode number matches cached temp file. */\n> +\t\treturn s.st_ino == inodeNr_;\n> +\t}\n> +\n> +\tint fd_;\n> +\tino_t inodeNr_;\n> +\tFileDescriptor *desc1_, *desc2_;\n> +};\n> +\n> +TEST_REGISTER(FileDescriptorTest)\n> diff --git a/test/meson.build b/test/meson.build\n> index 1bb2161dc05a7b1d..9dc392df30efd956 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -25,6 +25,7 @@ internal_tests = [\n>      ['event',                           'event.cpp'],\n>      ['event-dispatcher',                'event-dispatcher.cpp'],\n>      ['event-thread',                    'event-thread.cpp'],\n> +    ['file_descriptor',                 'file_descriptor.cpp'],\n>      ['message',                         'message.cpp'],\n>      ['object',                          'object.cpp'],\n>      ['object-invoke',                   'object-invoke.cpp'],","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 ADED660473\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2019 03:10:42 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 211B59C5;\n\tTue, 17 Dec 2019 03:10:42 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1576548642;\n\tbh=KvvTtzmxqtBC9v8sOHz+K+d0u2DX6glyhT+Z915+S28=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=giLqJC33YnBsX9QAsGHw/eR4duIiYjzvZyHe05nUv+IA7oJ27AEpmwnFDthwIX6Ig\n\texPhRI3TesNJ9Kv4iYV7xU3J1fWE0OC7F5ARB0ho+IR6tmhGaIx1JHHtAZXV8DEojd\n\tePvoCBjLERnO27tdGj4A7SB1FdrEYP+WsIXbPNCQ=","Date":"Tue, 17 Dec 2019 04:10:32 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191217021032.GP4856@pendragon.ideasonboard.com>","References":"<20191217015106.1175515-1-niklas.soderlund@ragnatech.se>\n\t<20191217015106.1175515-4-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191217015106.1175515-4-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: file_descriptor: Add test","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>","X-List-Received-Date":"Tue, 17 Dec 2019 02:10:42 -0000"}},{"id":3320,"web_url":"https://patchwork.libcamera.org/comment/3320/","msgid":"<20200104181834.GA4906@pendragon.ideasonboard.com>","date":"2020-01-04T18:18:34","subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: file_descriptor: Add test","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOne additional comment.\n\nOn Tue, Dec 17, 2019 at 04:10:32AM +0200, Laurent Pinchart wrote:\n> On Tue, Dec 17, 2019 at 02:51:06AM +0100, Niklas Söderlund wrote:\n> > Add a test which exercises the whole FileDescriptor interface.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  test/file_descriptor.cpp | 165 +++++++++++++++++++++++++++++++++++++++\n\nI would rename this to file-descriptor.cpp to be consistent with the\nother tests.\n\n> >  test/meson.build         |   1 +\n> >  2 files changed, 166 insertions(+)\n> >  create mode 100644 test/file_descriptor.cpp\n> > \n> > diff --git a/test/file_descriptor.cpp b/test/file_descriptor.cpp\n> > new file mode 100644\n> > index 0000000000000000..177d8bcf16591185\n> > --- /dev/null\n> > +++ b/test/file_descriptor.cpp\n> > @@ -0,0 +1,165 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * file_descriptor.cpp - FileDescriptor test\n> > + */\n> > +\n> > +#include <fcntl.h>\n> > +#include <sys/stat.h>\n> > +#include <sys/types.h>\n> > +#include <unistd.h>\n> > +\n> > +#include <libcamera/file_descriptor.h>\n> > +\n> > +#include \"test.h\"\n> > +\n> > +using namespace libcamera;\n> > +using namespace std;\n> > +\n> > +class FileDescriptorTest : public Test\n> > +{\n> > +protected:\n> > +\tint init()\n> > +\t{\n> > +\t\tdesc1_ = nullptr;\n> > +\t\tdesc2_ = nullptr;\n> > +\n> > +\t\tfd_ = open(\"/tmp\", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);\n> > +\t\tif (fd_ < 0)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/* Cache inode number of temp file. */\n> > +\t\tstruct stat s;\n> > +\t\tif (fstat(fd_, &s))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tinodeNr_ = s.st_ino;\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint run()\n> > +\t{\n> > +\t\t/* Test creating FileDescriptor from numerical file descriptor. */\n> > +\t\tdesc1_ = new FileDescriptor(fd_);\n> > +\t\tif (desc1_->fd() == fd_)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!isValidFd(fd_) || !isValidFd(desc1_->fd()))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tint fd = desc1_->fd();\n> > +\n> > +\t\tdelete desc1_;\n> > +\t\tdesc1_ = nullptr;\n> > +\n> > +\t\tif (!isValidFd(fd_) || isValidFd(fd))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/* Test creating FileDescriptor from other FileDescriptor. */\n> > +\t\tdesc1_ = new FileDescriptor(fd_);\n> > +\t\tdesc2_ = new FileDescriptor(*desc1_);\n> > +\n> > +\t\tif (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() == desc2_->fd())\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tdelete desc1_;\n> > +\t\tdesc1_ = nullptr;\n> > +\n> > +\t\tif (!isValidFd(desc2_->fd()))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tdelete desc2_;\n> > +\t\tdesc2_ = nullptr;\n> > +\n> > +\t\t/* Test creating FileDescriptor by taking over other FileDescriptor. */\n> > +\t\tdesc1_ = new FileDescriptor(fd_);\n> > +\t\tfd = desc1_->fd();\n> > +\t\tdesc2_ = new FileDescriptor(std::move(*desc1_));\n> > +\n> > +\t\tif (desc1_->fd() != -1 || desc2_->fd() != fd)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!isValidFd(desc2_->fd()))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tdelete desc1_;\n> > +\t\tdesc1_ = nullptr;\n> > +\t\tdelete desc2_;\n> > +\t\tdesc2_ = nullptr;\n> > +\n> > +\t\t/* Test creating FileDescriptor by copy assignment. */\n> > +\t\tdesc1_ = new FileDescriptor();\n> > +\t\tdesc2_ = new FileDescriptor(fd_);\n> > +\n> > +\t\tif (desc1_->fd() != -1)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tfd = desc2_->fd();\n> > +\t\t*desc1_ = *desc2_;\n> > +\n> > +\t\tif (desc1_->fd() == fd || desc2_->fd() != fd)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tdelete desc1_;\n> > +\t\tdesc1_ = nullptr;\n> > +\t\tdelete desc2_;\n> > +\t\tdesc2_ = nullptr;\n> > +\n> > +\t\tif (!isValidFd(fd_))\n> > +\t\t\treturn TestFail;\n> \n> You can drop this check.\n> \n> > +\n> > +\t\t/* Test creating FileDescriptor by move assignment. */\n> > +\t\tdesc1_ = new FileDescriptor();\n> > +\t\tdesc2_ = new FileDescriptor(fd_);\n> > +\n> > +\t\tfd = desc2_->fd();\n> > +\t\t*desc1_ = std::move(*desc2_);\n> > +\n> > +\t\tif (desc1_->fd() != fd || desc2_->fd() != -1)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!isValidFd(desc1_->fd()))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tdelete desc1_;\n> > +\t\tdesc1_ = nullptr;\n> > +\t\tdelete desc2_;\n> > +\t\tdesc2_ = nullptr;\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tvoid cleanup()\n> > +\t{\n> > +\t\tdelete desc2_;\n> > +\t\tdelete desc1_;\n> > +\n> > +\t\tif (fd_ > 0)\n> > +\t\t\tclose(fd_);\n> > +\t}\n> > +\n> > +private:\n> > +\tbool isValidFd(int fd)\n> > +\t{\n> > +\t\tstruct stat s;\n> > +\t\tif (fstat(fd, &s))\n> > +\t\t\treturn false;\n> > +\n> > +\t\t/* Check that inode number matches cached temp file. */\n> > +\t\treturn s.st_ino == inodeNr_;\n> > +\t}\n> > +\n> > +\tint fd_;\n> > +\tino_t inodeNr_;\n> > +\tFileDescriptor *desc1_, *desc2_;\n> > +};\n> > +\n> > +TEST_REGISTER(FileDescriptorTest)\n> > diff --git a/test/meson.build b/test/meson.build\n> > index 1bb2161dc05a7b1d..9dc392df30efd956 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -25,6 +25,7 @@ internal_tests = [\n> >      ['event',                           'event.cpp'],\n> >      ['event-dispatcher',                'event-dispatcher.cpp'],\n> >      ['event-thread',                    'event-thread.cpp'],\n> > +    ['file_descriptor',                 'file_descriptor.cpp'],\n> >      ['message',                         'message.cpp'],\n> >      ['object',                          'object.cpp'],\n> >      ['object-invoke',                   'object-invoke.cpp'],","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 A0F28605EF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  4 Jan 2020 19:18:45 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 15926A31;\n\tSat,  4 Jan 2020 19:18:44 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578161925;\n\tbh=KiZYJayOc39SYj2qlX0ymxPqNfRQkKhQ/l6gy8wVk1Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SUqI/dVLE3zZ4esqE5kVoLTRZ3v2/v20Sd+2OdxTybaJbQg8dKywDSo9usA4azB/p\n\tKtqBQYXD5s+opW0ZWTZS1GB4DHpTU/uuX+ZoLi6K5jo8VebqLFv8nHvaAL1vDXKsJv\n\ttnic1hJw8y2eoKkbVL2BY+o6tdURLJtG5ZzFn/qs=","Date":"Sat, 4 Jan 2020 20:18:34 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200104181834.GA4906@pendragon.ideasonboard.com>","References":"<20191217015106.1175515-1-niklas.soderlund@ragnatech.se>\n\t<20191217015106.1175515-4-niklas.soderlund@ragnatech.se>\n\t<20191217021032.GP4856@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191217021032.GP4856@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: file_descriptor: Add test","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>","X-List-Received-Date":"Sat, 04 Jan 2020 18:18:45 -0000"}}]