[{"id":2061,"web_url":"https://patchwork.libcamera.org/comment/2061/","msgid":"<20190701001052.GN7043@pendragon.ideasonboard.com>","date":"2019-07-01T00:10:52","subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: ipc: unix: Add test for\n\tIPCUnixSocket","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 Sun, Jun 30, 2019 at 06:25:14PM +0200, Niklas Söderlund wrote:\n> Test that the IPC supports sending data and file descriptors over the\n> IPC medium. To be able to execute the test two parts are needed, one\n> to drive the test and act as the libcamera (master) and a one to act as\n> the IPA (slave).\n> \n> The master drives the testing posting requests to the slave to process\n> and sometimes respond to. A few different tests are performed.\n> \n> - Master sends an array to the slave which responds with a reversed copy\n>   of the array. The master verifies that a reversed array is returned.\n> \n> - Master sends a list of file descriptors and ask the slave to calculate\n>   and respond with the sum of the size of the files. The master verifies\n>   that the calculated size is correct.\n> \n> - Master sends a pre-computed size and a list of file descriptors and\n>   ask the slave to verify that the pre-computed size matches the sum of\n\ns/ask/asks/\n\n>   the size of the file descriptors.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  test/ipc/meson.build    |  12 ++\n>  test/ipc/unixsocket.cpp | 390 ++++++++++++++++++++++++++++++++++++++++\n>  test/meson.build        |   1 +\n>  3 files changed, 403 insertions(+)\n>  create mode 100644 test/ipc/meson.build\n>  create mode 100644 test/ipc/unixsocket.cpp\n> \n> diff --git a/test/ipc/meson.build b/test/ipc/meson.build\n> new file mode 100644\n> index 0000000000000000..ca8375f35df91731\n> --- /dev/null\n> +++ b/test/ipc/meson.build\n> @@ -0,0 +1,12 @@\n> +ipc_tests = [\n> +    [ 'unixsocket',  'unixsocket.cpp' ],\n> +]\n> +\n> +foreach t : ipc_tests\n> +    exe = executable(t[0], t[1],\n> +                     dependencies : libcamera_dep,\n> +                     link_with : test_libraries,\n> +                     include_directories : test_includes_internal)\n> +\n> +    test(t[0], exe, suite : 'ipc', is_parallel : false)\n> +endforeach\n> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> new file mode 100644\n> index 0000000000000000..f433102af1b6d2a3\n> --- /dev/null\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -0,0 +1,390 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * unixsocket.cpp - Unix socket IPC test\n> + */\n> +\n> +#include <algorithm>\n> +#include <atomic>\n> +#include <fcntl.h>\n> +#include <iostream>\n> +#include <string.h>\n> +#include <sys/wait.h>\n> +#include <unistd.h>\n> +\n> +#include <libcamera/camera_manager.h>\n> +#include <libcamera/event_dispatcher.h>\n> +#include <libcamera/timer.h>\n> +\n> +#include \"ipc_unixsocket.h\"\n> +#include \"test.h\"\n> +\n> +#define CMD_CLOSE\t0\n> +#define CMD_REVERSE\t1\n> +#define CMD_LEN_CALC\t2\n> +#define CMD_LEN_CMP\t3\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +int calculateLength(int fd)\n> +{\n> +\tlseek(fd, 0, 0);\n> +\tint size = lseek(fd, 0, SEEK_END);\n> +\tlseek(fd, 0, 0);\n> +\n> +\treturn size;\n> +}\n> +\n> +class UnixSocketTestSlave\n> +{\n> +public:\n> +\tUnixSocketTestSlave()\n> +\t\t: exitCode_(EXIT_FAILURE), exit_(false)\n> +\t{\n> +\t\tdispatcher_ = CameraManager::instance()->eventDispatcher();\n> +\t\tipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead);\n> +\t}\n> +\n> +\tint run(int fd)\n> +\t{\n> +\t\tif (ipc_.bind(fd)) {\n> +\t\t\tcerr << \"Failed to connect to IPC channel\" << endl;\n> +\t\t\treturn EXIT_FAILURE;\n> +\t\t}\n> +\n> +\t\twhile (!exit_)\n> +\t\t\tdispatcher_->processEvents();\n> +\n> +\t\tipc_.close();\n> +\n> +\t\treturn exitCode_;\n> +\t}\n> +\n> +private:\n> +\tvoid readyRead(IPCUnixSocket *ipc)\n> +\t{\n> +\t\tIPCUnixSocket::Payload message, response;\n> +\t\tint ret;\n> +\n> +\t\tif (ipc->receive(&message)) {\n> +\t\t\tcerr << \"Receive message failed\" << endl;\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\tconst uint8_t cmd = message.data[0];\n> +\t\tcout << \"Slave received command \" << static_cast<unsigned int>(cmd) << endl;\n> +\n> +\t\tswitch (cmd) {\n> +\t\tcase CMD_CLOSE:\n> +\t\t\tstop(0);\n> +\t\t\tbreak;\n> +\n> +\t\tcase CMD_REVERSE: {\n> +\t\t\tresponse.data = message.data;\n> +\t\t\tstd::reverse(response.data.begin() + 1, response.data.end());\n> +\n> +\t\t\tret = ipc_.send(response);\n> +\t\t\tif (ret < 0) {\n> +\t\t\t\tcerr << \"Reverse fail\" << endl;\n> +\t\t\t\tstop(ret);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tcase CMD_LEN_CALC: {\n> +\t\t\tint size = 0;\n> +\t\t\tfor (int fd : message.fds)\n> +\t\t\t\tsize += calculateLength(fd);\n> +\n> +\t\t\tresponse.data.resize(1 + sizeof(size));\n> +\t\t\tresponse.data[0] = cmd;\n> +\t\t\tmemcpy(response.data.data() + 1, &size, sizeof(size));\n> +\n> +\t\t\tret = ipc_.send(response);\n> +\t\t\tif (ret < 0) {\n> +\t\t\t\tcerr << \"Calc fail\" << endl;\n> +\t\t\t\tstop(ret);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tcase CMD_LEN_CMP: {\n> +\t\t\tint size = 0;\n> +\t\t\tfor (int fd : message.fds)\n> +\t\t\t\tsize += calculateLength(fd);\n> +\n> +\t\t\tint cmp;\n> +\t\t\tmemcpy(&cmp, message.data.data() + 1, sizeof(cmp));\n> +\n> +\t\t\tif (cmp != size) {\n> +\t\t\t\tcerr << \"Compare fail\" << endl;\n> +\t\t\t\tstop(-ERANGE);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tdefault:\n> +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> +\t\t\tstop(-EINVAL);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\tvoid stop(int code)\n> +\t{\n> +\t\texitCode_ = code;\n> +\t\texit_ = true;\n> +\t}\n> +\n> +\tIPCUnixSocket ipc_;\n> +\tEventDispatcher *dispatcher_;\n> +\tint exitCode_;\n> +\tbool exit_;\n> +};\n> +\n> +class UnixSocketTest : public Test\n> +{\n> +protected:\n> +\tint slaveStart(int fd)\n> +\t{\n> +\t\tpid_ = fork();\n> +\n> +\t\tif (pid_ == -1)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!pid_) {\n> +\t\t\tstd::string arg = std::to_string(fd);\n> +\t\t\texecl(\"/proc/self/exe\", \"/proc/self/exe\",\n> +\t\t\t      arg.c_str(), nullptr);\n> +\n> +\t\t\t/* Only get here if exec fails. */\n> +\t\t\texit(TestFail);\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint slaveStop()\n> +\t{\n> +\t\tint status;\n> +\n> +\t\tif (pid_ < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (waitpid(pid_, &status, 0) < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint testReverse()\n> +\t{\n> +\t\tIPCUnixSocket::Payload message, response;\n> +\t\tint ret;\n> +\n> +\t\tmessage.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };\n> +\n> +\t\tret = call(message, &response);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\tstd::reverse(response.data.begin() + 1, response.data.end());\n> +\t\tif (message.data != response.data)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint testCalc()\n> +\t{\n> +\t\tIPCUnixSocket::Payload message, response;\n> +\t\tint sizeOut, sizeIn, ret;\n> +\n> +\t\tsizeOut = prepareFDs(&message, 2);\n> +\t\tif (sizeOut < 0)\n> +\t\t\treturn sizeOut;\n> +\n> +\t\tmessage.data.push_back(CMD_LEN_CALC);\n> +\n> +\t\tret = call(message, &response);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\tmemcpy(&sizeIn, response.data.data() + 1, sizeof(sizeIn));\n> +\t\tif (sizeOut != sizeIn)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint testCmp()\n> +\t{\n> +\t\tIPCUnixSocket::Payload message;\n> +\t\tint size;\n> +\n> +\t\tsize = prepareFDs(&message, 7);\n> +\t\tif (size < 0)\n> +\t\t\treturn size;\n> +\n> +\t\tmessage.data.resize(1 + sizeof(size));\n> +\t\tmessage.data[0] = CMD_LEN_CMP;\n> +\t\tmemcpy(message.data.data() + 1, &size, sizeof(size));\n> +\n> +\t\tif (ipc_.send(message))\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint init()\n> +\t{\n> +\t\tcallResponse_ = nullptr;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\tint slavefd = ipc_.create();\n> +\t\tif (slavefd < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (slaveStart(slavefd)) {\n> +\t\t\tcerr << \"Failed to start slave\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tipc_.readyRead.connect(this, &UnixSocketTest::readyRead);\n> +\n> +\t\t/* Test reversing a string, this test sending only data. */\n> +\t\tif (testReverse()) {\n> +\t\t\tcerr << \"Reveres array test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test offloading a calculation, this test sending only FDs. */\n> +\t\tif (testCalc()) {\n> +\t\t\tcerr << \"Calc test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test fire and forget, this tests sending data and FDs. */\n> +\t\tif (testCmp()) {\n> +\t\t\tcerr << \"Cmp test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Close slave connection. */\n> +\t\tIPCUnixSocket::Payload close;\n> +\t\tclose.data.push_back(CMD_CLOSE);\n> +\t\tif (ipc_.send(close)) {\n> +\t\t\tcerr << \"Closing IPC channel failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tipc_.close();\n> +\t\tif (slaveStop()) {\n> +\t\t\tcerr << \"Failed to stop slave\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +private:\n> +\tint call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response)\n> +\t{\n> +\t\tTimer timeout;\n> +\t\tint ret;\n> +\n> +\t\tif (callResponse_)\n> +\t\t\treturn -EBUSY;\n\nI think you can skip this as all return paths of this function set\ncallResponse_ to nullptr.\n\n> +\n> +\t\tcallDone_ = false;\n> +\t\tcallResponse_ = response;\n> +\n> +\t\tret = ipc_.send(message);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\ttimeout.start(200);\n> +\t\twhile (!callDone_) {\n> +\t\t\tif (!timeout.isRunning()) {\n> +\t\t\t\tcerr << \"Call timeout!\" << endl;\n> +\t\t\t\tcallResponse_ = nullptr;\n> +\t\t\t\treturn -ETIMEDOUT;\n> +\t\t\t}\n> +\n> +\t\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> +\t\t}\n> +\n> +\t\tcallResponse_ = nullptr;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tvoid readyRead(IPCUnixSocket *ipc)\n> +\t{\n> +\t\tIPCUnixSocket::Payload message;\n\nUnused variable. Didn't the compiler warn ?\n\n> +\n> +\t\tif (!callResponse_) {\n> +\t\t\tcerr << \"Read ready without expecting data, fail.\" << endl;\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\tif (ipc->receive(callResponse_)) {\n> +\t\t\tcerr << \"Receive message failed\" << endl;\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\tconst uint8_t cmd = callResponse_->data[0];\n> +\t\tcout << \"Master received command \" << static_cast<unsigned int>(cmd) << endl;\n\nI would drop this print to avoid outputting message when the test\nsucceeds.\n\n> +\n> +\t\tcallDone_ = true;\n> +\t}\n> +\n> +\tint prepareFDs(IPCUnixSocket::Payload *message, unsigned int num)\n> +\t{\n> +\t\tint fd = open(\"/proc/self/exe\", O_RDONLY);\n> +\t\tif (fd < 0)\n> +\t\t\treturn fd;\n> +\n> +\t\tint size = 0;\n> +\t\tfor (unsigned int i = 0; i < num; i++) {\n> +\t\t\tint clone = dup(fd);\n> +\t\t\tif (clone < 0)\n> +\t\t\t\treturn clone;\n> +\n> +\t\t\tsize += calculateLength(clone);\n> +\t\t\tmessage->fds.push_back(clone);\n> +\t\t}\n\nI still think it would be nicer to use mkstemp() to create temporary\nfiles, and test their content instead of just the size. You could then\ndrop one of the two fd-related tests, and for instance create 2\ntemporary files with data, send their fds, and receive back the fd of a\nnew temporary file that contains the concatenation of the two.\n\n> +\n> +\t\tclose(fd);\n> +\n> +\t\treturn size;\n> +\t}\n> +\n> +\tpid_t pid_;\n> +\tIPCUnixSocket ipc_;\n> +\tbool callDone_;\n> +\tIPCUnixSocket::Payload *callResponse_;\n> +};\n> +\n> +/*\n> + * Can't use TEST_REGISTER() as single binary needs to act as both proxy\n> + * master and slave.\n> + */\n> +int main(int argc, char **argv)\n> +{\n> +\tif (argc == 2) {\n> +\t\tint ipcfd = std::stoi(argv[1]);\n> +\t\tUnixSocketTestSlave slave;\n> +\t\treturn slave.run(ipcfd);\n> +\t}\n> +\n> +\treturn UnixSocketTest().execute();\n> +}\n> diff --git a/test/meson.build b/test/meson.build\n> index c36ac24796367501..3666f6b2385bd4ca 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -2,6 +2,7 @@ subdir('libtest')\n>  \n>  subdir('camera')\n>  subdir('ipa')\n> +subdir('ipc')\n>  subdir('media_device')\n>  subdir('pipeline')\n>  subdir('stream')","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 7946B60BC0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 02:11:20 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4E17255;\n\tMon,  1 Jul 2019 02:11:19 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561939880;\n\tbh=jID6Fv4K6htmsPPybZpbq/Dyzf+uIU9HtoBWxNEFH6I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H7NDXVtqYgN8qSk3W18ofu5H9TVqohSf/bSsEnUWi4AJXSC2kMeLPgocGlEx+kPm3\n\tqNH9wP7eU0bhLyXO26LykzdFC8+cWbBQXJ+jsDemmgIqPhI35sVLoCCCcGhi/DMySO\n\tDcsPEvrPgS3gMNFWsQNTiLS37kdxEC5X1F6ItJwk=","Date":"Mon, 1 Jul 2019 03:10:52 +0300","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":"<20190701001052.GN7043@pendragon.ideasonboard.com>","References":"<20190630162514.20522-1-niklas.soderlund@ragnatech.se>\n\t<20190630162514.20522-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":"<20190630162514.20522-4-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: ipc: unix: Add test for\n\tIPCUnixSocket","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, 01 Jul 2019 00:11:20 -0000"}},{"id":2068,"web_url":"https://patchwork.libcamera.org/comment/2068/","msgid":"<20190701092726.GA19884@bigcity.dyn.berto.se>","date":"2019-07-01T09:27:26","subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: ipc: unix: Add test for\n\tIPCUnixSocket","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-07-01 03:10:52 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Sun, Jun 30, 2019 at 06:25:14PM +0200, Niklas Söderlund wrote:\n> > Test that the IPC supports sending data and file descriptors over the\n> > IPC medium. To be able to execute the test two parts are needed, one\n> > to drive the test and act as the libcamera (master) and a one to act as\n> > the IPA (slave).\n> > \n> > The master drives the testing posting requests to the slave to process\n> > and sometimes respond to. A few different tests are performed.\n> > \n> > - Master sends an array to the slave which responds with a reversed copy\n> >   of the array. The master verifies that a reversed array is returned.\n> > \n> > - Master sends a list of file descriptors and ask the slave to calculate\n> >   and respond with the sum of the size of the files. The master verifies\n> >   that the calculated size is correct.\n> > \n> > - Master sends a pre-computed size and a list of file descriptors and\n> >   ask the slave to verify that the pre-computed size matches the sum of\n> \n> s/ask/asks/\n> \n> >   the size of the file descriptors.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  test/ipc/meson.build    |  12 ++\n> >  test/ipc/unixsocket.cpp | 390 ++++++++++++++++++++++++++++++++++++++++\n> >  test/meson.build        |   1 +\n> >  3 files changed, 403 insertions(+)\n> >  create mode 100644 test/ipc/meson.build\n> >  create mode 100644 test/ipc/unixsocket.cpp\n> > \n> > diff --git a/test/ipc/meson.build b/test/ipc/meson.build\n> > new file mode 100644\n> > index 0000000000000000..ca8375f35df91731\n> > --- /dev/null\n> > +++ b/test/ipc/meson.build\n> > @@ -0,0 +1,12 @@\n> > +ipc_tests = [\n> > +    [ 'unixsocket',  'unixsocket.cpp' ],\n> > +]\n> > +\n> > +foreach t : ipc_tests\n> > +    exe = executable(t[0], t[1],\n> > +                     dependencies : libcamera_dep,\n> > +                     link_with : test_libraries,\n> > +                     include_directories : test_includes_internal)\n> > +\n> > +    test(t[0], exe, suite : 'ipc', is_parallel : false)\n> > +endforeach\n> > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> > new file mode 100644\n> > index 0000000000000000..f433102af1b6d2a3\n> > --- /dev/null\n> > +++ b/test/ipc/unixsocket.cpp\n> > @@ -0,0 +1,390 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * unixsocket.cpp - Unix socket IPC test\n> > + */\n> > +\n> > +#include <algorithm>\n> > +#include <atomic>\n> > +#include <fcntl.h>\n> > +#include <iostream>\n> > +#include <string.h>\n> > +#include <sys/wait.h>\n> > +#include <unistd.h>\n> > +\n> > +#include <libcamera/camera_manager.h>\n> > +#include <libcamera/event_dispatcher.h>\n> > +#include <libcamera/timer.h>\n> > +\n> > +#include \"ipc_unixsocket.h\"\n> > +#include \"test.h\"\n> > +\n> > +#define CMD_CLOSE\t0\n> > +#define CMD_REVERSE\t1\n> > +#define CMD_LEN_CALC\t2\n> > +#define CMD_LEN_CMP\t3\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +int calculateLength(int fd)\n> > +{\n> > +\tlseek(fd, 0, 0);\n> > +\tint size = lseek(fd, 0, SEEK_END);\n> > +\tlseek(fd, 0, 0);\n> > +\n> > +\treturn size;\n> > +}\n> > +\n> > +class UnixSocketTestSlave\n> > +{\n> > +public:\n> > +\tUnixSocketTestSlave()\n> > +\t\t: exitCode_(EXIT_FAILURE), exit_(false)\n> > +\t{\n> > +\t\tdispatcher_ = CameraManager::instance()->eventDispatcher();\n> > +\t\tipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead);\n> > +\t}\n> > +\n> > +\tint run(int fd)\n> > +\t{\n> > +\t\tif (ipc_.bind(fd)) {\n> > +\t\t\tcerr << \"Failed to connect to IPC channel\" << endl;\n> > +\t\t\treturn EXIT_FAILURE;\n> > +\t\t}\n> > +\n> > +\t\twhile (!exit_)\n> > +\t\t\tdispatcher_->processEvents();\n> > +\n> > +\t\tipc_.close();\n> > +\n> > +\t\treturn exitCode_;\n> > +\t}\n> > +\n> > +private:\n> > +\tvoid readyRead(IPCUnixSocket *ipc)\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload message, response;\n> > +\t\tint ret;\n> > +\n> > +\t\tif (ipc->receive(&message)) {\n> > +\t\t\tcerr << \"Receive message failed\" << endl;\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\n> > +\t\tconst uint8_t cmd = message.data[0];\n> > +\t\tcout << \"Slave received command \" << static_cast<unsigned int>(cmd) << endl;\n> > +\n> > +\t\tswitch (cmd) {\n> > +\t\tcase CMD_CLOSE:\n> > +\t\t\tstop(0);\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcase CMD_REVERSE: {\n> > +\t\t\tresponse.data = message.data;\n> > +\t\t\tstd::reverse(response.data.begin() + 1, response.data.end());\n> > +\n> > +\t\t\tret = ipc_.send(response);\n> > +\t\t\tif (ret < 0) {\n> > +\t\t\t\tcerr << \"Reverse fail\" << endl;\n> > +\t\t\t\tstop(ret);\n> > +\t\t\t}\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\n> > +\t\tcase CMD_LEN_CALC: {\n> > +\t\t\tint size = 0;\n> > +\t\t\tfor (int fd : message.fds)\n> > +\t\t\t\tsize += calculateLength(fd);\n> > +\n> > +\t\t\tresponse.data.resize(1 + sizeof(size));\n> > +\t\t\tresponse.data[0] = cmd;\n> > +\t\t\tmemcpy(response.data.data() + 1, &size, sizeof(size));\n> > +\n> > +\t\t\tret = ipc_.send(response);\n> > +\t\t\tif (ret < 0) {\n> > +\t\t\t\tcerr << \"Calc fail\" << endl;\n> > +\t\t\t\tstop(ret);\n> > +\t\t\t}\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\n> > +\t\tcase CMD_LEN_CMP: {\n> > +\t\t\tint size = 0;\n> > +\t\t\tfor (int fd : message.fds)\n> > +\t\t\t\tsize += calculateLength(fd);\n> > +\n> > +\t\t\tint cmp;\n> > +\t\t\tmemcpy(&cmp, message.data.data() + 1, sizeof(cmp));\n> > +\n> > +\t\t\tif (cmp != size) {\n> > +\t\t\t\tcerr << \"Compare fail\" << endl;\n> > +\t\t\t\tstop(-ERANGE);\n> > +\t\t\t}\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\n> > +\t\tdefault:\n> > +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> > +\t\t\tstop(-EINVAL);\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tvoid stop(int code)\n> > +\t{\n> > +\t\texitCode_ = code;\n> > +\t\texit_ = true;\n> > +\t}\n> > +\n> > +\tIPCUnixSocket ipc_;\n> > +\tEventDispatcher *dispatcher_;\n> > +\tint exitCode_;\n> > +\tbool exit_;\n> > +};\n> > +\n> > +class UnixSocketTest : public Test\n> > +{\n> > +protected:\n> > +\tint slaveStart(int fd)\n> > +\t{\n> > +\t\tpid_ = fork();\n> > +\n> > +\t\tif (pid_ == -1)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!pid_) {\n> > +\t\t\tstd::string arg = std::to_string(fd);\n> > +\t\t\texecl(\"/proc/self/exe\", \"/proc/self/exe\",\n> > +\t\t\t      arg.c_str(), nullptr);\n> > +\n> > +\t\t\t/* Only get here if exec fails. */\n> > +\t\t\texit(TestFail);\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint slaveStop()\n> > +\t{\n> > +\t\tint status;\n> > +\n> > +\t\tif (pid_ < 0)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (waitpid(pid_, &status, 0) < 0)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint testReverse()\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload message, response;\n> > +\t\tint ret;\n> > +\n> > +\t\tmessage.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };\n> > +\n> > +\t\tret = call(message, &response);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\tstd::reverse(response.data.begin() + 1, response.data.end());\n> > +\t\tif (message.data != response.data)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint testCalc()\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload message, response;\n> > +\t\tint sizeOut, sizeIn, ret;\n> > +\n> > +\t\tsizeOut = prepareFDs(&message, 2);\n> > +\t\tif (sizeOut < 0)\n> > +\t\t\treturn sizeOut;\n> > +\n> > +\t\tmessage.data.push_back(CMD_LEN_CALC);\n> > +\n> > +\t\tret = call(message, &response);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\tmemcpy(&sizeIn, response.data.data() + 1, sizeof(sizeIn));\n> > +\t\tif (sizeOut != sizeIn)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint testCmp()\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload message;\n> > +\t\tint size;\n> > +\n> > +\t\tsize = prepareFDs(&message, 7);\n> > +\t\tif (size < 0)\n> > +\t\t\treturn size;\n> > +\n> > +\t\tmessage.data.resize(1 + sizeof(size));\n> > +\t\tmessage.data[0] = CMD_LEN_CMP;\n> > +\t\tmemcpy(message.data.data() + 1, &size, sizeof(size));\n> > +\n> > +\t\tif (ipc_.send(message))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint init()\n> > +\t{\n> > +\t\tcallResponse_ = nullptr;\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint run()\n> > +\t{\n> > +\t\tint slavefd = ipc_.create();\n> > +\t\tif (slavefd < 0)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (slaveStart(slavefd)) {\n> > +\t\t\tcerr << \"Failed to start slave\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tipc_.readyRead.connect(this, &UnixSocketTest::readyRead);\n> > +\n> > +\t\t/* Test reversing a string, this test sending only data. */\n> > +\t\tif (testReverse()) {\n> > +\t\t\tcerr << \"Reveres array test failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test offloading a calculation, this test sending only FDs. */\n> > +\t\tif (testCalc()) {\n> > +\t\t\tcerr << \"Calc test failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test fire and forget, this tests sending data and FDs. */\n> > +\t\tif (testCmp()) {\n> > +\t\t\tcerr << \"Cmp test failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Close slave connection. */\n> > +\t\tIPCUnixSocket::Payload close;\n> > +\t\tclose.data.push_back(CMD_CLOSE);\n> > +\t\tif (ipc_.send(close)) {\n> > +\t\t\tcerr << \"Closing IPC channel failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tipc_.close();\n> > +\t\tif (slaveStop()) {\n> > +\t\t\tcerr << \"Failed to stop slave\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +private:\n> > +\tint call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response)\n> > +\t{\n> > +\t\tTimer timeout;\n> > +\t\tint ret;\n> > +\n> > +\t\tif (callResponse_)\n> > +\t\t\treturn -EBUSY;\n> \n> I think you can skip this as all return paths of this function set\n> callResponse_ to nullptr.\n> \n> > +\n> > +\t\tcallDone_ = false;\n> > +\t\tcallResponse_ = response;\n> > +\n> > +\t\tret = ipc_.send(message);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\ttimeout.start(200);\n> > +\t\twhile (!callDone_) {\n> > +\t\t\tif (!timeout.isRunning()) {\n> > +\t\t\t\tcerr << \"Call timeout!\" << endl;\n> > +\t\t\t\tcallResponse_ = nullptr;\n> > +\t\t\t\treturn -ETIMEDOUT;\n> > +\t\t\t}\n> > +\n> > +\t\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> > +\t\t}\n> > +\n> > +\t\tcallResponse_ = nullptr;\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tvoid readyRead(IPCUnixSocket *ipc)\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload message;\n> \n> Unused variable. Didn't the compiler warn ?\n\nNope, does yours?\n\nI have never received any help from g++ nor clang when it comes to \nwarning about unused none basic data types, I'm starting to believe it's \nnot possible due to this could have an effect as IPCUnixSocket::Payload \nconstructor could do a lot of things which are intentional even tho the \nvariable itself is not used.\n\nIn the past I been able to shake out a few unused basic data type \nvariables from loops when comping with -O0, but that do not catch this \nvariable for me.\n\n> \n> > +\n> > +\t\tif (!callResponse_) {\n> > +\t\t\tcerr << \"Read ready without expecting data, fail.\" << endl;\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\n> > +\t\tif (ipc->receive(callResponse_)) {\n> > +\t\t\tcerr << \"Receive message failed\" << endl;\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\n> > +\t\tconst uint8_t cmd = callResponse_->data[0];\n> > +\t\tcout << \"Master received command \" << static_cast<unsigned int>(cmd) << endl;\n> \n> I would drop this print to avoid outputting message when the test\n> succeeds.\n> \n> > +\n> > +\t\tcallDone_ = true;\n> > +\t}\n> > +\n> > +\tint prepareFDs(IPCUnixSocket::Payload *message, unsigned int num)\n> > +\t{\n> > +\t\tint fd = open(\"/proc/self/exe\", O_RDONLY);\n> > +\t\tif (fd < 0)\n> > +\t\t\treturn fd;\n> > +\n> > +\t\tint size = 0;\n> > +\t\tfor (unsigned int i = 0; i < num; i++) {\n> > +\t\t\tint clone = dup(fd);\n> > +\t\t\tif (clone < 0)\n> > +\t\t\t\treturn clone;\n> > +\n> > +\t\t\tsize += calculateLength(clone);\n> > +\t\t\tmessage->fds.push_back(clone);\n> > +\t\t}\n> \n> I still think it would be nicer to use mkstemp() to create temporary\n> files, and test their content instead of just the size. You could then\n> drop one of the two fd-related tests, and for instance create 2\n> temporary files with data, send their fds, and receive back the fd of a\n> new temporary file that contains the concatenation of the two.\n\nI think this one is nicer :-)\n\nI'm quiet confident that file IO works in the system, what I'm not so \nconfident in is that the IPC mechanism is able to pass multiple FDs.  \nWith this construct we test passing 2 and 7 file descriptors and verify \nthat the FDs are valid by the most simple operation on them. Adding a \ntest which you describe would indeed test a similar thing but IMHO in a \nmore complex way with no benefit to the IPC verification.\n\nAlso as soon as we start creating temporary files in a TC we also need \nto care more about cleanup logic. As we don't wish to leave waste after \nus, with this construct it's not an issue.\n\n> \n> > +\n> > +\t\tclose(fd);\n> > +\n> > +\t\treturn size;\n> > +\t}\n> > +\n> > +\tpid_t pid_;\n> > +\tIPCUnixSocket ipc_;\n> > +\tbool callDone_;\n> > +\tIPCUnixSocket::Payload *callResponse_;\n> > +};\n> > +\n> > +/*\n> > + * Can't use TEST_REGISTER() as single binary needs to act as both proxy\n> > + * master and slave.\n> > + */\n> > +int main(int argc, char **argv)\n> > +{\n> > +\tif (argc == 2) {\n> > +\t\tint ipcfd = std::stoi(argv[1]);\n> > +\t\tUnixSocketTestSlave slave;\n> > +\t\treturn slave.run(ipcfd);\n> > +\t}\n> > +\n> > +\treturn UnixSocketTest().execute();\n> > +}\n> > diff --git a/test/meson.build b/test/meson.build\n> > index c36ac24796367501..3666f6b2385bd4ca 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -2,6 +2,7 @@ subdir('libtest')\n> >  \n> >  subdir('camera')\n> >  subdir('ipa')\n> > +subdir('ipc')\n> >  subdir('media_device')\n> >  subdir('pipeline')\n> >  subdir('stream')\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EBC6461950\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 11:27:28 +0200 (CEST)","by mail-lf1-x144.google.com with SMTP id u10so8252927lfm.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 01 Jul 2019 02:27:28 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tk15sm3198273lji.89.2019.07.01.02.27.27\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tMon, 01 Jul 2019 02:27:27 -0700 (PDT)"],"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=CF1EoT3KJ8l1JZGkXTrCNtvge7Li2rhXP4+ylwqBTik=;\n\tb=sOUTP73NKU+haIJkjU7xwYLsSMt89bIovuBWL5rbHQ+CFgVpoEWfXu5Fom2bbvFUvl\n\t7m27wpGoPc76W5a6aCW7WVyMIWRVTBlEeoml1blF83ytxFjQvdYQYBhE6Lt0vycplMsQ\n\tjBCCMqTL9JstjSXubfmIXIK3plkHjYSYq/aAo1WMbR7oDqzxxnunU5jt68rFr2mmJ5A4\n\t9571k0rCMv5RxAjdIJgynSRBZRvt+8lcIvLc1tKNahA1sygpmbL+Sz5krBG7Du0FYocg\n\t9G3kd39MzOw3M+Ib/7uPMnGlhBtcFxY+P2EgT0kyt/uSgL9oiqPsh34p6daxjTkf/Atd\n\t+mIA==","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=CF1EoT3KJ8l1JZGkXTrCNtvge7Li2rhXP4+ylwqBTik=;\n\tb=UlYkqBqtOj5Ha01TRDMog/WmDbNDIP0aTMUyvEMZFlqtDr1yvkw67VA7e5PS2geQ/h\n\tmanMs40PQfAVG81H7kRLUTO+ksoWC+7VgQOwjHuQsfg70LXdmLYghjrnGzC5aSM+Qfld\n\toZSZ3fGpoftY6aWANvRhqm2p306/nIPJhm68asH+JZHfYfbdoviWQVEAJRDeM34LeUPw\n\t/V3rZcZgTao1qVkq88jA0/fMpVRQTpjyc4QeHogYhssvOoXXs08an+q6QTcRizYFw4cI\n\tnL0qv/X/eWOC/0gSqizUZKC3ER7O6Yqn+3Q4xV1O1kzDgvf5YO3u/m79mmq+akKcatJw\n\thbKA==","X-Gm-Message-State":"APjAAAV2Gpu4ucTyEsNjk7c8adrT9tN1di5YLfMkD34YcsETaobXQy4E\n\t0nOjF9tyjEKtlvRt7GCHB9vsWxvgL+4=","X-Google-Smtp-Source":"APXvYqzqixveTkR+Dy5nl3+7aoM7/8oJP3EqqwotkEOzEnV2lO5X/cxwKt0ilL27b4h0IRQa8sFY3A==","X-Received":"by 2002:ac2:44a4:: with SMTP id c4mr7039588lfm.116.1561973247793;\n\tMon, 01 Jul 2019 02:27:27 -0700 (PDT)","Date":"Mon, 1 Jul 2019 11:27:26 +0200","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","Message-ID":"<20190701092726.GA19884@bigcity.dyn.berto.se>","References":"<20190630162514.20522-1-niklas.soderlund@ragnatech.se>\n\t<20190630162514.20522-4-niklas.soderlund@ragnatech.se>\n\t<20190701001052.GN7043@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190701001052.GN7043@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: ipc: unix: Add test for\n\tIPCUnixSocket","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, 01 Jul 2019 09:27:29 -0000"}},{"id":2069,"web_url":"https://patchwork.libcamera.org/comment/2069/","msgid":"<20190701101529.GA5018@pendragon.ideasonboard.com>","date":"2019-07-01T10:15:29","subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: ipc: unix: Add test for\n\tIPCUnixSocket","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Mon, Jul 01, 2019 at 11:27:26AM +0200, Niklas Söderlund wrote:\n> On 2019-07-01 03:10:52 +0300, Laurent Pinchart wrote:\n> > On Sun, Jun 30, 2019 at 06:25:14PM +0200, Niklas Söderlund wrote:\n> >> Test that the IPC supports sending data and file descriptors over the\n> >> IPC medium. To be able to execute the test two parts are needed, one\n> >> to drive the test and act as the libcamera (master) and a one to act as\n> >> the IPA (slave).\n> >> \n> >> The master drives the testing posting requests to the slave to process\n> >> and sometimes respond to. A few different tests are performed.\n> >> \n> >> - Master sends an array to the slave which responds with a reversed copy\n> >>   of the array. The master verifies that a reversed array is returned.\n> >> \n> >> - Master sends a list of file descriptors and ask the slave to calculate\n> >>   and respond with the sum of the size of the files. The master verifies\n> >>   that the calculated size is correct.\n> >> \n> >> - Master sends a pre-computed size and a list of file descriptors and\n> >>   ask the slave to verify that the pre-computed size matches the sum of\n> > \n> > s/ask/asks/\n> > \n> >>   the size of the file descriptors.\n> >> \n> >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >> ---\n> >>  test/ipc/meson.build    |  12 ++\n> >>  test/ipc/unixsocket.cpp | 390 ++++++++++++++++++++++++++++++++++++++++\n> >>  test/meson.build        |   1 +\n> >>  3 files changed, 403 insertions(+)\n> >>  create mode 100644 test/ipc/meson.build\n> >>  create mode 100644 test/ipc/unixsocket.cpp\n> >> \n> >> diff --git a/test/ipc/meson.build b/test/ipc/meson.build\n> >> new file mode 100644\n> >> index 0000000000000000..ca8375f35df91731\n> >> --- /dev/null\n> >> +++ b/test/ipc/meson.build\n> >> @@ -0,0 +1,12 @@\n> >> +ipc_tests = [\n> >> +    [ 'unixsocket',  'unixsocket.cpp' ],\n> >> +]\n> >> +\n> >> +foreach t : ipc_tests\n> >> +    exe = executable(t[0], t[1],\n> >> +                     dependencies : libcamera_dep,\n> >> +                     link_with : test_libraries,\n> >> +                     include_directories : test_includes_internal)\n> >> +\n> >> +    test(t[0], exe, suite : 'ipc', is_parallel : false)\n> >> +endforeach\n> >> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> >> new file mode 100644\n> >> index 0000000000000000..f433102af1b6d2a3\n> >> --- /dev/null\n> >> +++ b/test/ipc/unixsocket.cpp\n> >> @@ -0,0 +1,390 @@\n> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> >> +/*\n> >> + * Copyright (C) 2019, Google Inc.\n> >> + *\n> >> + * unixsocket.cpp - Unix socket IPC test\n> >> + */\n> >> +\n> >> +#include <algorithm>\n> >> +#include <atomic>\n> >> +#include <fcntl.h>\n> >> +#include <iostream>\n> >> +#include <string.h>\n> >> +#include <sys/wait.h>\n> >> +#include <unistd.h>\n> >> +\n> >> +#include <libcamera/camera_manager.h>\n> >> +#include <libcamera/event_dispatcher.h>\n> >> +#include <libcamera/timer.h>\n> >> +\n> >> +#include \"ipc_unixsocket.h\"\n> >> +#include \"test.h\"\n> >> +\n> >> +#define CMD_CLOSE\t0\n> >> +#define CMD_REVERSE\t1\n> >> +#define CMD_LEN_CALC\t2\n> >> +#define CMD_LEN_CMP\t3\n> >> +\n> >> +using namespace std;\n> >> +using namespace libcamera;\n> >> +\n> >> +int calculateLength(int fd)\n> >> +{\n> >> +\tlseek(fd, 0, 0);\n> >> +\tint size = lseek(fd, 0, SEEK_END);\n> >> +\tlseek(fd, 0, 0);\n> >> +\n> >> +\treturn size;\n> >> +}\n> >> +\n> >> +class UnixSocketTestSlave\n> >> +{\n> >> +public:\n> >> +\tUnixSocketTestSlave()\n> >> +\t\t: exitCode_(EXIT_FAILURE), exit_(false)\n> >> +\t{\n> >> +\t\tdispatcher_ = CameraManager::instance()->eventDispatcher();\n> >> +\t\tipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead);\n> >> +\t}\n> >> +\n> >> +\tint run(int fd)\n> >> +\t{\n> >> +\t\tif (ipc_.bind(fd)) {\n> >> +\t\t\tcerr << \"Failed to connect to IPC channel\" << endl;\n> >> +\t\t\treturn EXIT_FAILURE;\n> >> +\t\t}\n> >> +\n> >> +\t\twhile (!exit_)\n> >> +\t\t\tdispatcher_->processEvents();\n> >> +\n> >> +\t\tipc_.close();\n> >> +\n> >> +\t\treturn exitCode_;\n> >> +\t}\n> >> +\n> >> +private:\n> >> +\tvoid readyRead(IPCUnixSocket *ipc)\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload message, response;\n> >> +\t\tint ret;\n> >> +\n> >> +\t\tif (ipc->receive(&message)) {\n> >> +\t\t\tcerr << \"Receive message failed\" << endl;\n> >> +\t\t\treturn;\n> >> +\t\t}\n> >> +\n> >> +\t\tconst uint8_t cmd = message.data[0];\n> >> +\t\tcout << \"Slave received command \" << static_cast<unsigned int>(cmd) << endl;\n> >> +\n> >> +\t\tswitch (cmd) {\n> >> +\t\tcase CMD_CLOSE:\n> >> +\t\t\tstop(0);\n> >> +\t\t\tbreak;\n> >> +\n> >> +\t\tcase CMD_REVERSE: {\n> >> +\t\t\tresponse.data = message.data;\n> >> +\t\t\tstd::reverse(response.data.begin() + 1, response.data.end());\n> >> +\n> >> +\t\t\tret = ipc_.send(response);\n> >> +\t\t\tif (ret < 0) {\n> >> +\t\t\t\tcerr << \"Reverse fail\" << endl;\n> >> +\t\t\t\tstop(ret);\n> >> +\t\t\t}\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\n> >> +\t\tcase CMD_LEN_CALC: {\n> >> +\t\t\tint size = 0;\n> >> +\t\t\tfor (int fd : message.fds)\n> >> +\t\t\t\tsize += calculateLength(fd);\n> >> +\n> >> +\t\t\tresponse.data.resize(1 + sizeof(size));\n> >> +\t\t\tresponse.data[0] = cmd;\n> >> +\t\t\tmemcpy(response.data.data() + 1, &size, sizeof(size));\n> >> +\n> >> +\t\t\tret = ipc_.send(response);\n> >> +\t\t\tif (ret < 0) {\n> >> +\t\t\t\tcerr << \"Calc fail\" << endl;\n> >> +\t\t\t\tstop(ret);\n> >> +\t\t\t}\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\n> >> +\t\tcase CMD_LEN_CMP: {\n> >> +\t\t\tint size = 0;\n> >> +\t\t\tfor (int fd : message.fds)\n> >> +\t\t\t\tsize += calculateLength(fd);\n> >> +\n> >> +\t\t\tint cmp;\n> >> +\t\t\tmemcpy(&cmp, message.data.data() + 1, sizeof(cmp));\n> >> +\n> >> +\t\t\tif (cmp != size) {\n> >> +\t\t\t\tcerr << \"Compare fail\" << endl;\n> >> +\t\t\t\tstop(-ERANGE);\n> >> +\t\t\t}\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\n> >> +\t\tdefault:\n> >> +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> >> +\t\t\tstop(-EINVAL);\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\t}\n> >> +\n> >> +\tvoid stop(int code)\n> >> +\t{\n> >> +\t\texitCode_ = code;\n> >> +\t\texit_ = true;\n> >> +\t}\n> >> +\n> >> +\tIPCUnixSocket ipc_;\n> >> +\tEventDispatcher *dispatcher_;\n> >> +\tint exitCode_;\n> >> +\tbool exit_;\n> >> +};\n> >> +\n> >> +class UnixSocketTest : public Test\n> >> +{\n> >> +protected:\n> >> +\tint slaveStart(int fd)\n> >> +\t{\n> >> +\t\tpid_ = fork();\n> >> +\n> >> +\t\tif (pid_ == -1)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (!pid_) {\n> >> +\t\t\tstd::string arg = std::to_string(fd);\n> >> +\t\t\texecl(\"/proc/self/exe\", \"/proc/self/exe\",\n> >> +\t\t\t      arg.c_str(), nullptr);\n> >> +\n> >> +\t\t\t/* Only get here if exec fails. */\n> >> +\t\t\texit(TestFail);\n> >> +\t\t}\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\tint slaveStop()\n> >> +\t{\n> >> +\t\tint status;\n> >> +\n> >> +\t\tif (pid_ < 0)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (waitpid(pid_, &status, 0) < 0)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\tint testReverse()\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload message, response;\n> >> +\t\tint ret;\n> >> +\n> >> +\t\tmessage.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };\n> >> +\n> >> +\t\tret = call(message, &response);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\n> >> +\t\tstd::reverse(response.data.begin() + 1, response.data.end());\n> >> +\t\tif (message.data != response.data)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tint testCalc()\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload message, response;\n> >> +\t\tint sizeOut, sizeIn, ret;\n> >> +\n> >> +\t\tsizeOut = prepareFDs(&message, 2);\n> >> +\t\tif (sizeOut < 0)\n> >> +\t\t\treturn sizeOut;\n> >> +\n> >> +\t\tmessage.data.push_back(CMD_LEN_CALC);\n> >> +\n> >> +\t\tret = call(message, &response);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\n> >> +\t\tmemcpy(&sizeIn, response.data.data() + 1, sizeof(sizeIn));\n> >> +\t\tif (sizeOut != sizeIn)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tint testCmp()\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload message;\n> >> +\t\tint size;\n> >> +\n> >> +\t\tsize = prepareFDs(&message, 7);\n> >> +\t\tif (size < 0)\n> >> +\t\t\treturn size;\n> >> +\n> >> +\t\tmessage.data.resize(1 + sizeof(size));\n> >> +\t\tmessage.data[0] = CMD_LEN_CMP;\n> >> +\t\tmemcpy(message.data.data() + 1, &size, sizeof(size));\n> >> +\n> >> +\t\tif (ipc_.send(message))\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tint init()\n> >> +\t{\n> >> +\t\tcallResponse_ = nullptr;\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tint run()\n> >> +\t{\n> >> +\t\tint slavefd = ipc_.create();\n> >> +\t\tif (slavefd < 0)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (slaveStart(slavefd)) {\n> >> +\t\t\tcerr << \"Failed to start slave\" << endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\tipc_.readyRead.connect(this, &UnixSocketTest::readyRead);\n> >> +\n> >> +\t\t/* Test reversing a string, this test sending only data. */\n> >> +\t\tif (testReverse()) {\n> >> +\t\t\tcerr << \"Reveres array test failed\" << endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\t/* Test offloading a calculation, this test sending only FDs. */\n> >> +\t\tif (testCalc()) {\n> >> +\t\t\tcerr << \"Calc test failed\" << endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\t/* Test fire and forget, this tests sending data and FDs. */\n> >> +\t\tif (testCmp()) {\n> >> +\t\t\tcerr << \"Cmp test failed\" << endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\t/* Close slave connection. */\n> >> +\t\tIPCUnixSocket::Payload close;\n> >> +\t\tclose.data.push_back(CMD_CLOSE);\n> >> +\t\tif (ipc_.send(close)) {\n> >> +\t\t\tcerr << \"Closing IPC channel failed\" << endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\tipc_.close();\n> >> +\t\tif (slaveStop()) {\n> >> +\t\t\tcerr << \"Failed to stop slave\" << endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +private:\n> >> +\tint call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response)\n> >> +\t{\n> >> +\t\tTimer timeout;\n> >> +\t\tint ret;\n> >> +\n> >> +\t\tif (callResponse_)\n> >> +\t\t\treturn -EBUSY;\n> > \n> > I think you can skip this as all return paths of this function set\n> > callResponse_ to nullptr.\n> > \n> >> +\n> >> +\t\tcallDone_ = false;\n> >> +\t\tcallResponse_ = response;\n> >> +\n> >> +\t\tret = ipc_.send(message);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\n> >> +\t\ttimeout.start(200);\n> >> +\t\twhile (!callDone_) {\n> >> +\t\t\tif (!timeout.isRunning()) {\n> >> +\t\t\t\tcerr << \"Call timeout!\" << endl;\n> >> +\t\t\t\tcallResponse_ = nullptr;\n> >> +\t\t\t\treturn -ETIMEDOUT;\n> >> +\t\t\t}\n> >> +\n> >> +\t\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> >> +\t\t}\n> >> +\n> >> +\t\tcallResponse_ = nullptr;\n> >> +\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tvoid readyRead(IPCUnixSocket *ipc)\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload message;\n> > \n> > Unused variable. Didn't the compiler warn ?\n> \n> Nope, does yours?\n\nIt usually does, I haven't checked here.\n\n> I have never received any help from g++ nor clang when it comes to \n> warning about unused none basic data types, I'm starting to believe it's \n> not possible due to this could have an effect as IPCUnixSocket::Payload \n> constructor could do a lot of things which are intentional even tho the \n> variable itself is not used.\n> \n> In the past I been able to shake out a few unused basic data type \n> variables from loops when comping with -O0, but that do not catch this \n> variable for me.\n> \n> >> +\n> >> +\t\tif (!callResponse_) {\n> >> +\t\t\tcerr << \"Read ready without expecting data, fail.\" << endl;\n> >> +\t\t\treturn;\n> >> +\t\t}\n> >> +\n> >> +\t\tif (ipc->receive(callResponse_)) {\n> >> +\t\t\tcerr << \"Receive message failed\" << endl;\n> >> +\t\t\treturn;\n> >> +\t\t}\n> >> +\n> >> +\t\tconst uint8_t cmd = callResponse_->data[0];\n> >> +\t\tcout << \"Master received command \" << static_cast<unsigned int>(cmd) << endl;\n> > \n> > I would drop this print to avoid outputting message when the test\n> > succeeds.\n> > \n> >> +\n> >> +\t\tcallDone_ = true;\n> >> +\t}\n> >> +\n> >> +\tint prepareFDs(IPCUnixSocket::Payload *message, unsigned int num)\n> >> +\t{\n> >> +\t\tint fd = open(\"/proc/self/exe\", O_RDONLY);\n> >> +\t\tif (fd < 0)\n> >> +\t\t\treturn fd;\n> >> +\n> >> +\t\tint size = 0;\n> >> +\t\tfor (unsigned int i = 0; i < num; i++) {\n> >> +\t\t\tint clone = dup(fd);\n> >> +\t\t\tif (clone < 0)\n> >> +\t\t\t\treturn clone;\n> >> +\n> >> +\t\t\tsize += calculateLength(clone);\n> >> +\t\t\tmessage->fds.push_back(clone);\n> >> +\t\t}\n> > \n> > I still think it would be nicer to use mkstemp() to create temporary\n> > files, and test their content instead of just the size. You could then\n> > drop one of the two fd-related tests, and for instance create 2\n> > temporary files with data, send their fds, and receive back the fd of a\n> > new temporary file that contains the concatenation of the two.\n> \n> I think this one is nicer :-)\n> \n> I'm quiet confident that file IO works in the system, what I'm not so \n> confident in is that the IPC mechanism is able to pass multiple FDs.  \n> With this construct we test passing 2 and 7 file descriptors and verify \n> that the FDs are valid by the most simple operation on them. Adding a \n> test which you describe would indeed test a similar thing but IMHO in a \n> more complex way with no benefit to the IPC verification.\n\nMy concern here is that by passing multiple fds refering to the same\nfile, you may indeed be testing passing multiple fds, but you can't\nverify the ordering :-S\n\n> Also as soon as we start creating temporary files in a TC we also need \n> to care more about cleanup logic. As we don't wish to leave waste after \n> us, with this construct it's not an issue.\n\nI was thinking you could unlink the file as soon as it gets created, but\nanother option would be to use shm_open() to avoid creating files on\ndisk.\n\n> >> +\n> >> +\t\tclose(fd);\n> >> +\n> >> +\t\treturn size;\n> >> +\t}\n> >> +\n> >> +\tpid_t pid_;\n> >> +\tIPCUnixSocket ipc_;\n> >> +\tbool callDone_;\n> >> +\tIPCUnixSocket::Payload *callResponse_;\n> >> +};\n> >> +\n> >> +/*\n> >> + * Can't use TEST_REGISTER() as single binary needs to act as both proxy\n> >> + * master and slave.\n> >> + */\n> >> +int main(int argc, char **argv)\n> >> +{\n> >> +\tif (argc == 2) {\n> >> +\t\tint ipcfd = std::stoi(argv[1]);\n> >> +\t\tUnixSocketTestSlave slave;\n> >> +\t\treturn slave.run(ipcfd);\n> >> +\t}\n> >> +\n> >> +\treturn UnixSocketTest().execute();\n> >> +}\n> >> diff --git a/test/meson.build b/test/meson.build\n> >> index c36ac24796367501..3666f6b2385bd4ca 100644\n> >> --- a/test/meson.build\n> >> +++ b/test/meson.build\n> >> @@ -2,6 +2,7 @@ subdir('libtest')\n> >>  \n> >>  subdir('camera')\n> >>  subdir('ipa')\n> >> +subdir('ipc')\n> >>  subdir('media_device')\n> >>  subdir('pipeline')\n> >>  subdir('stream')","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 2C6AD619E5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 12:15:49 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A29EE524;\n\tMon,  1 Jul 2019 12:15:48 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561976148;\n\tbh=YcUgyfTM95yXjkCnvfterK+HBHMoUyuuMHCgKA0sY9o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rXxosqHwFw/Xw3DWJiMmAetd2rkoEUUzFZ7ElvYPB7RhAq4InCYbeBGowD+fPlqlF\n\tFQM8DcG0260kZBDLwp4QjrXWLtWVmAqx2e0VvYHis4LZ3zWN0eP4y+QVhqaQWR+j+I\n\tQMZVEW83ZnX+b5mb0W1jXm8+Tk5HzMidnahrkJs0=","Date":"Mon, 1 Jul 2019 13:15:29 +0300","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":"<20190701101529.GA5018@pendragon.ideasonboard.com>","References":"<20190630162514.20522-1-niklas.soderlund@ragnatech.se>\n\t<20190630162514.20522-4-niklas.soderlund@ragnatech.se>\n\t<20190701001052.GN7043@pendragon.ideasonboard.com>\n\t<20190701092726.GA19884@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190701092726.GA19884@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: ipc: unix: Add test for\n\tIPCUnixSocket","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, 01 Jul 2019 10:15:49 -0000"}},{"id":2079,"web_url":"https://patchwork.libcamera.org/comment/2079/","msgid":"<20190701163611.GA11004@bigcity.dyn.berto.se>","date":"2019-07-01T16:36:11","subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: ipc: unix: Add test for\n\tIPCUnixSocket","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-07-01 13:15:29 +0300, Laurent Pinchart wrote:\n> > >> +\tint prepareFDs(IPCUnixSocket::Payload *message, unsigned \n> > >> int num)\n> > >> +\t{\n> > >> +\t\tint fd = open(\"/proc/self/exe\", O_RDONLY);\n> > >> +\t\tif (fd < 0)\n> > >> +\t\t\treturn fd;\n> > >> +\n> > >> +\t\tint size = 0;\n> > >> +\t\tfor (unsigned int i = 0; i < num; i++) {\n> > >> +\t\t\tint clone = dup(fd);\n> > >> +\t\t\tif (clone < 0)\n> > >> +\t\t\t\treturn clone;\n> > >> +\n> > >> +\t\t\tsize += calculateLength(clone);\n> > >> +\t\t\tmessage->fds.push_back(clone);\n> > >> +\t\t}\n> > > \n> > > I still think it would be nicer to use mkstemp() to create temporary\n> > > files, and test their content instead of just the size. You could then\n> > > drop one of the two fd-related tests, and for instance create 2\n> > > temporary files with data, send their fds, and receive back the fd of a\n> > > new temporary file that contains the concatenation of the two.\n> > \n> > I think this one is nicer :-)\n> > \n> > I'm quiet confident that file IO works in the system, what I'm not so \n> > confident in is that the IPC mechanism is able to pass multiple FDs.  \n> > With this construct we test passing 2 and 7 file descriptors and verify \n> > that the FDs are valid by the most simple operation on them. Adding a \n> > test which you describe would indeed test a similar thing but IMHO in a \n> > more complex way with no benefit to the IPC verification.\n> \n> My concern here is that by passing multiple fds refering to the same\n> file, you may indeed be testing passing multiple fds, but you can't\n> verify the ordering :-S\n\nThis is a reason I can agree with. I will add a test to verify ordering \nof fds, but I will keep the use of dup() here as it tests transferring \nmay fds.\n\n> \n> > Also as soon as we start creating temporary files in a TC we also need \n> > to care more about cleanup logic. As we don't wish to leave waste after \n> > us, with this construct it's not an issue.\n> \n> I was thinking you could unlink the file as soon as it gets created, but\n> another option would be to use shm_open() to avoid creating files on\n> disk.\n\nNice tip, I will try to make use of shm_open() in the next version for \nthe additional test.","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 089AF60BF8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 18:36:14 +0200 (CEST)","by mail-lf1-x142.google.com with SMTP id z15so9217890lfh.13\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 01 Jul 2019 09:36:13 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tm4sm3371495ljc.56.2019.07.01.09.36.11\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tMon, 01 Jul 2019 09:36:12 -0700 (PDT)"],"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=bvlYlCRsfvmqZaJpjTfapUt5TNFxwHHhY6UhBA+dtrM=;\n\tb=FSsx6OfNbxOENfzaP/ceYgFWeuiaDdhWFT29KfVJMJP11sjHbeXCvfuANw7QptDY8c\n\tvw0UVrvQaw+IQ+xet1c0Fm2a2zOUXeUcCEHf4uX5CGCkVinAFMmnzX3ZKCG7eKNqkKy5\n\tzaeMbqRCrulrpJm0fp2evTcSC5SOC832VYdU4O3pezTinqceMLoxlBL6f0jm5IUg+jvX\n\tDL/YRjsYP1Z+E/bdE4DGjDDX6YVV8+R+qBE6kFzoT0dP2IzuX9I2Y0sV/GteR2fMTWIB\n\tGOcCLMM6QNxV9aI3Htmo4kwv0slo6csNwUdUTibF2fAM/tbVSvrQiNhT34XGu4bs4daK\n\tl5jg==","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=bvlYlCRsfvmqZaJpjTfapUt5TNFxwHHhY6UhBA+dtrM=;\n\tb=P0YPxsZLF0q3kmbquqqTEcatsp7T6H1Jp+cjlpy9WA1shYZlMlzcGnH5CyGU6Oe7Tn\n\tTE1ZBF54Y6gBSQrt0sH+czdMBUUVAJ6s14X4mWKXUHtpGt3XP/CiCv4S5XPNRCVcBGiT\n\t0/CwGSOW135LUBrwYfTDGzlNFRsMa5jDuGBo/TrA1l7tWYPyv3DMSUWLoFXvbq/A2Ugv\n\t2mh9+DW5iy29Wl8R3DcUNVqQuaxcVvlJPrno+K3tC9KTAu4tZ79ymguv/ArrL4PV/QGu\n\t/Fny+yrXA+YYbZng7fiHc3cxMc9xKS+DRjtqhhrBDgxihmH6fIce60ho8PAbxHR3JSJN\n\tGjkQ==","X-Gm-Message-State":"APjAAAXScw4UR1dzA11KyMfxlzrauiUddLYvNb46L2Q1dabm1EhstjDK\n\ti9Q+npoAize2n5bKUvvs41hhqg==","X-Google-Smtp-Source":"APXvYqwdpDxb306llJDVhBCceUliunuZgDKaylO4XIz/YEQdLKJG8LuwBsvTKoTx2EvCdrpguLu9Ng==","X-Received":"by 2002:ac2:5324:: with SMTP id\n\tf4mr12326494lfh.156.1561998973141; \n\tMon, 01 Jul 2019 09:36:13 -0700 (PDT)","Date":"Mon, 1 Jul 2019 18:36:11 +0200","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","Message-ID":"<20190701163611.GA11004@bigcity.dyn.berto.se>","References":"<20190630162514.20522-1-niklas.soderlund@ragnatech.se>\n\t<20190630162514.20522-4-niklas.soderlund@ragnatech.se>\n\t<20190701001052.GN7043@pendragon.ideasonboard.com>\n\t<20190701092726.GA19884@bigcity.dyn.berto.se>\n\t<20190701101529.GA5018@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190701101529.GA5018@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] test: ipc: unix: Add test for\n\tIPCUnixSocket","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, 01 Jul 2019 16:36:14 -0000"}}]