[libcamera-devel,v2,3/3] test: ipc: unix: Add test for IPCUnixSocket

1532 diff mbox series

Test that the IPC supports sending data and file descriptors over the
IPC medium. To be able to execute the test two parts are needed, one
to drive the test and act as the libcamera (master) and a one to act as
the IPA (slave).

The master drives the testing posting requests to the slave to process
and sometimes respond to. A few different tests are performed.

- Master sends an array to the slave which responds with a reversed copy
  of the array. The master verifies that a reversed array is returned.

- Master sends a list of file descriptors and ask the slave to calculate
  and respond with the sum of the size of the files. The master verifies
  that the calculated size is correct.

- Master sends a pre-computed size and a list of file descriptors and
  ask the slave to verify that the pre-computed size matches the sum of
  the size of the file descriptors.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 test/ipc/meson.build    |  12 ++
 test/ipc/unixsocket.cpp | 390 ++++++++++++++++++++++++++++++++++++++++
 test/meson.build        |   1 +
 3 files changed, 403 insertions(+)
 create mode 100644 test/ipc/meson.build
 create mode 100644 test/ipc/unixsocket.cpp

Hi Niklas,

Thank you for the patch.

On Sun, Jun 30, 2019 at 06:25:14PM +0200, Niklas Söderlund wrote:
> Test that the IPC supports sending data and file descriptors over the
> IPC medium. To be able to execute the test two parts are needed, one
> to drive the test and act as the libcamera (master) and a one to act as
> the IPA (slave).
> 
> The master drives the testing posting requests to the slave to process
> and sometimes respond to. A few different tests are performed.
> 
> - Master sends an array to the slave which responds with a reversed copy
>   of the array. The master verifies that a reversed array is returned.
> 
> - Master sends a list of file descriptors and ask the slave to calculate
>   and respond with the sum of the size of the files. The master verifies
>   that the calculated size is correct.
> 
> - Master sends a pre-computed size and a list of file descriptors and
>   ask the slave to verify that the pre-computed size matches the sum of

s/ask/asks/

>   the size of the file descriptors.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  test/ipc/meson.build    |  12 ++
>  test/ipc/unixsocket.cpp | 390 ++++++++++++++++++++++++++++++++++++++++
>  test/meson.build        |   1 +
>  3 files changed, 403 insertions(+)
>  create mode 100644 test/ipc/meson.build
>  create mode 100644 test/ipc/unixsocket.cpp
> 
> diff --git a/test/ipc/meson.build b/test/ipc/meson.build
> new file mode 100644
> index 0000000000000000..ca8375f35df91731
> --- /dev/null
> +++ b/test/ipc/meson.build
> @@ -0,0 +1,12 @@
> +ipc_tests = [
> +    [ 'unixsocket',  'unixsocket.cpp' ],
> +]
> +
> +foreach t : ipc_tests
> +    exe = executable(t[0], t[1],
> +                     dependencies : libcamera_dep,
> +                     link_with : test_libraries,
> +                     include_directories : test_includes_internal)
> +
> +    test(t[0], exe, suite : 'ipc', is_parallel : false)
> +endforeach
> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp
> new file mode 100644
> index 0000000000000000..f433102af1b6d2a3
> --- /dev/null
> +++ b/test/ipc/unixsocket.cpp
> @@ -0,0 +1,390 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * unixsocket.cpp - Unix socket IPC test
> + */
> +
> +#include <algorithm>
> +#include <atomic>
> +#include <fcntl.h>
> +#include <iostream>
> +#include <string.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +
> +#include <libcamera/camera_manager.h>
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/timer.h>
> +
> +#include "ipc_unixsocket.h"
> +#include "test.h"
> +
> +#define CMD_CLOSE	0
> +#define CMD_REVERSE	1
> +#define CMD_LEN_CALC	2
> +#define CMD_LEN_CMP	3
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +int calculateLength(int fd)
> +{
> +	lseek(fd, 0, 0);
> +	int size = lseek(fd, 0, SEEK_END);
> +	lseek(fd, 0, 0);
> +
> +	return size;
> +}
> +
> +class UnixSocketTestSlave
> +{
> +public:
> +	UnixSocketTestSlave()
> +		: exitCode_(EXIT_FAILURE), exit_(false)
> +	{
> +		dispatcher_ = CameraManager::instance()->eventDispatcher();
> +		ipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead);
> +	}
> +
> +	int run(int fd)
> +	{
> +		if (ipc_.bind(fd)) {
> +			cerr << "Failed to connect to IPC channel" << endl;
> +			return EXIT_FAILURE;
> +		}
> +
> +		while (!exit_)
> +			dispatcher_->processEvents();
> +
> +		ipc_.close();
> +
> +		return exitCode_;
> +	}
> +
> +private:
> +	void readyRead(IPCUnixSocket *ipc)
> +	{
> +		IPCUnixSocket::Payload message, response;
> +		int ret;
> +
> +		if (ipc->receive(&message)) {
> +			cerr << "Receive message failed" << endl;
> +			return;
> +		}
> +
> +		const uint8_t cmd = message.data[0];
> +		cout << "Slave received command " << static_cast<unsigned int>(cmd) << endl;
> +
> +		switch (cmd) {
> +		case CMD_CLOSE:
> +			stop(0);
> +			break;
> +
> +		case CMD_REVERSE: {
> +			response.data = message.data;
> +			std::reverse(response.data.begin() + 1, response.data.end());
> +
> +			ret = ipc_.send(response);
> +			if (ret < 0) {
> +				cerr << "Reverse fail" << endl;
> +				stop(ret);
> +			}
> +			break;
> +		}
> +
> +		case CMD_LEN_CALC: {
> +			int size = 0;
> +			for (int fd : message.fds)
> +				size += calculateLength(fd);
> +
> +			response.data.resize(1 + sizeof(size));
> +			response.data[0] = cmd;
> +			memcpy(response.data.data() + 1, &size, sizeof(size));
> +
> +			ret = ipc_.send(response);
> +			if (ret < 0) {
> +				cerr << "Calc fail" << endl;
> +				stop(ret);
> +			}
> +			break;
> +		}
> +
> +		case CMD_LEN_CMP: {
> +			int size = 0;
> +			for (int fd : message.fds)
> +				size += calculateLength(fd);
> +
> +			int cmp;
> +			memcpy(&cmp, message.data.data() + 1, sizeof(cmp));
> +
> +			if (cmp != size) {
> +				cerr << "Compare fail" << endl;
> +				stop(-ERANGE);
> +			}
> +			break;
> +		}
> +
> +		default:
> +			cerr << "Unknown command " << cmd << endl;
> +			stop(-EINVAL);
> +			break;
> +		}
> +	}
> +
> +	void stop(int code)
> +	{
> +		exitCode_ = code;
> +		exit_ = true;
> +	}
> +
> +	IPCUnixSocket ipc_;
> +	EventDispatcher *dispatcher_;
> +	int exitCode_;
> +	bool exit_;
> +};
> +
> +class UnixSocketTest : public Test
> +{
> +protected:
> +	int slaveStart(int fd)
> +	{
> +		pid_ = fork();
> +
> +		if (pid_ == -1)
> +			return TestFail;
> +
> +		if (!pid_) {
> +			std::string arg = std::to_string(fd);
> +			execl("/proc/self/exe", "/proc/self/exe",
> +			      arg.c_str(), nullptr);
> +
> +			/* Only get here if exec fails. */
> +			exit(TestFail);
> +		}
> +
> +		return TestPass;
> +	}
> +
> +	int slaveStop()
> +	{
> +		int status;
> +
> +		if (pid_ < 0)
> +			return TestFail;
> +
> +		if (waitpid(pid_, &status, 0) < 0)
> +			return TestFail;
> +
> +		if (!WIFEXITED(status) || WEXITSTATUS(status))
> +			return TestFail;
> +
> +		return TestPass;
> +	}
> +
> +	int testReverse()
> +	{
> +		IPCUnixSocket::Payload message, response;
> +		int ret;
> +
> +		message.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };
> +
> +		ret = call(message, &response);
> +		if (ret)
> +			return ret;
> +
> +		std::reverse(response.data.begin() + 1, response.data.end());
> +		if (message.data != response.data)
> +			return TestFail;
> +
> +		return 0;
> +	}
> +
> +	int testCalc()
> +	{
> +		IPCUnixSocket::Payload message, response;
> +		int sizeOut, sizeIn, ret;
> +
> +		sizeOut = prepareFDs(&message, 2);
> +		if (sizeOut < 0)
> +			return sizeOut;
> +
> +		message.data.push_back(CMD_LEN_CALC);
> +
> +		ret = call(message, &response);
> +		if (ret)
> +			return ret;
> +
> +		memcpy(&sizeIn, response.data.data() + 1, sizeof(sizeIn));
> +		if (sizeOut != sizeIn)
> +			return TestFail;
> +
> +		return 0;
> +	}
> +
> +	int testCmp()
> +	{
> +		IPCUnixSocket::Payload message;
> +		int size;
> +
> +		size = prepareFDs(&message, 7);
> +		if (size < 0)
> +			return size;
> +
> +		message.data.resize(1 + sizeof(size));
> +		message.data[0] = CMD_LEN_CMP;
> +		memcpy(message.data.data() + 1, &size, sizeof(size));
> +
> +		if (ipc_.send(message))
> +			return TestFail;
> +
> +		return 0;
> +	}
> +
> +	int init()
> +	{
> +		callResponse_ = nullptr;
> +		return 0;
> +	}
> +
> +	int run()
> +	{
> +		int slavefd = ipc_.create();
> +		if (slavefd < 0)
> +			return TestFail;
> +
> +		if (slaveStart(slavefd)) {
> +			cerr << "Failed to start slave" << endl;
> +			return TestFail;
> +		}
> +
> +		ipc_.readyRead.connect(this, &UnixSocketTest::readyRead);
> +
> +		/* Test reversing a string, this test sending only data. */
> +		if (testReverse()) {
> +			cerr << "Reveres array test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Test offloading a calculation, this test sending only FDs. */
> +		if (testCalc()) {
> +			cerr << "Calc test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Test fire and forget, this tests sending data and FDs. */
> +		if (testCmp()) {
> +			cerr << "Cmp test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Close slave connection. */
> +		IPCUnixSocket::Payload close;
> +		close.data.push_back(CMD_CLOSE);
> +		if (ipc_.send(close)) {
> +			cerr << "Closing IPC channel failed" << endl;
> +			return TestFail;
> +		}
> +
> +		ipc_.close();
> +		if (slaveStop()) {
> +			cerr << "Failed to stop slave" << endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +
> +private:
> +	int call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response)
> +	{
> +		Timer timeout;
> +		int ret;
> +
> +		if (callResponse_)
> +			return -EBUSY;

I think you can skip this as all return paths of this function set
callResponse_ to nullptr.

> +
> +		callDone_ = false;
> +		callResponse_ = response;
> +
> +		ret = ipc_.send(message);
> +		if (ret)
> +			return ret;
> +
> +		timeout.start(200);
> +		while (!callDone_) {
> +			if (!timeout.isRunning()) {
> +				cerr << "Call timeout!" << endl;
> +				callResponse_ = nullptr;
> +				return -ETIMEDOUT;
> +			}
> +
> +			CameraManager::instance()->eventDispatcher()->processEvents();
> +		}
> +
> +		callResponse_ = nullptr;
> +
> +		return 0;
> +	}
> +
> +	void readyRead(IPCUnixSocket *ipc)
> +	{
> +		IPCUnixSocket::Payload message;

Unused variable. Didn't the compiler warn ?

> +
> +		if (!callResponse_) {
> +			cerr << "Read ready without expecting data, fail." << endl;
> +			return;
> +		}
> +
> +		if (ipc->receive(callResponse_)) {
> +			cerr << "Receive message failed" << endl;
> +			return;
> +		}
> +
> +		const uint8_t cmd = callResponse_->data[0];
> +		cout << "Master received command " << static_cast<unsigned int>(cmd) << endl;

I would drop this print to avoid outputting message when the test
succeeds.

> +
> +		callDone_ = true;
> +	}
> +
> +	int prepareFDs(IPCUnixSocket::Payload *message, unsigned int num)
> +	{
> +		int fd = open("/proc/self/exe", O_RDONLY);
> +		if (fd < 0)
> +			return fd;
> +
> +		int size = 0;
> +		for (unsigned int i = 0; i < num; i++) {
> +			int clone = dup(fd);
> +			if (clone < 0)
> +				return clone;
> +
> +			size += calculateLength(clone);
> +			message->fds.push_back(clone);
> +		}

I still think it would be nicer to use mkstemp() to create temporary
files, and test their content instead of just the size. You could then
drop one of the two fd-related tests, and for instance create 2
temporary files with data, send their fds, and receive back the fd of a
new temporary file that contains the concatenation of the two.

> +
> +		close(fd);
> +
> +		return size;
> +	}
> +
> +	pid_t pid_;
> +	IPCUnixSocket ipc_;
> +	bool callDone_;
> +	IPCUnixSocket::Payload *callResponse_;
> +};
> +
> +/*
> + * Can't use TEST_REGISTER() as single binary needs to act as both proxy
> + * master and slave.
> + */
> +int main(int argc, char **argv)
> +{
> +	if (argc == 2) {
> +		int ipcfd = std::stoi(argv[1]);
> +		UnixSocketTestSlave slave;
> +		return slave.run(ipcfd);
> +	}
> +
> +	return UnixSocketTest().execute();
> +}
> diff --git a/test/meson.build b/test/meson.build
> index c36ac24796367501..3666f6b2385bd4ca 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -2,6 +2,7 @@ subdir('libtest')
>  
>  subdir('camera')
>  subdir('ipa')
> +subdir('ipc')
>  subdir('media_device')
>  subdir('pipeline')
>  subdir('stream')

Hi Laurent,

Thanks for your feedback.

On 2019-07-01 03:10:52 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Sun, Jun 30, 2019 at 06:25:14PM +0200, Niklas Söderlund wrote:
> > Test that the IPC supports sending data and file descriptors over the
> > IPC medium. To be able to execute the test two parts are needed, one
> > to drive the test and act as the libcamera (master) and a one to act as
> > the IPA (slave).
> > 
> > The master drives the testing posting requests to the slave to process
> > and sometimes respond to. A few different tests are performed.
> > 
> > - Master sends an array to the slave which responds with a reversed copy
> >   of the array. The master verifies that a reversed array is returned.
> > 
> > - Master sends a list of file descriptors and ask the slave to calculate
> >   and respond with the sum of the size of the files. The master verifies
> >   that the calculated size is correct.
> > 
> > - Master sends a pre-computed size and a list of file descriptors and
> >   ask the slave to verify that the pre-computed size matches the sum of
> 
> s/ask/asks/
> 
> >   the size of the file descriptors.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  test/ipc/meson.build    |  12 ++
> >  test/ipc/unixsocket.cpp | 390 ++++++++++++++++++++++++++++++++++++++++
> >  test/meson.build        |   1 +
> >  3 files changed, 403 insertions(+)
> >  create mode 100644 test/ipc/meson.build
> >  create mode 100644 test/ipc/unixsocket.cpp
> > 
> > diff --git a/test/ipc/meson.build b/test/ipc/meson.build
> > new file mode 100644
> > index 0000000000000000..ca8375f35df91731
> > --- /dev/null
> > +++ b/test/ipc/meson.build
> > @@ -0,0 +1,12 @@
> > +ipc_tests = [
> > +    [ 'unixsocket',  'unixsocket.cpp' ],
> > +]
> > +
> > +foreach t : ipc_tests
> > +    exe = executable(t[0], t[1],
> > +                     dependencies : libcamera_dep,
> > +                     link_with : test_libraries,
> > +                     include_directories : test_includes_internal)
> > +
> > +    test(t[0], exe, suite : 'ipc', is_parallel : false)
> > +endforeach
> > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp
> > new file mode 100644
> > index 0000000000000000..f433102af1b6d2a3
> > --- /dev/null
> > +++ b/test/ipc/unixsocket.cpp
> > @@ -0,0 +1,390 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * unixsocket.cpp - Unix socket IPC test
> > + */
> > +
> > +#include <algorithm>
> > +#include <atomic>
> > +#include <fcntl.h>
> > +#include <iostream>
> > +#include <string.h>
> > +#include <sys/wait.h>
> > +#include <unistd.h>
> > +
> > +#include <libcamera/camera_manager.h>
> > +#include <libcamera/event_dispatcher.h>
> > +#include <libcamera/timer.h>
> > +
> > +#include "ipc_unixsocket.h"
> > +#include "test.h"
> > +
> > +#define CMD_CLOSE	0
> > +#define CMD_REVERSE	1
> > +#define CMD_LEN_CALC	2
> > +#define CMD_LEN_CMP	3
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +
> > +int calculateLength(int fd)
> > +{
> > +	lseek(fd, 0, 0);
> > +	int size = lseek(fd, 0, SEEK_END);
> > +	lseek(fd, 0, 0);
> > +
> > +	return size;
> > +}
> > +
> > +class UnixSocketTestSlave
> > +{
> > +public:
> > +	UnixSocketTestSlave()
> > +		: exitCode_(EXIT_FAILURE), exit_(false)
> > +	{
> > +		dispatcher_ = CameraManager::instance()->eventDispatcher();
> > +		ipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead);
> > +	}
> > +
> > +	int run(int fd)
> > +	{
> > +		if (ipc_.bind(fd)) {
> > +			cerr << "Failed to connect to IPC channel" << endl;
> > +			return EXIT_FAILURE;
> > +		}
> > +
> > +		while (!exit_)
> > +			dispatcher_->processEvents();
> > +
> > +		ipc_.close();
> > +
> > +		return exitCode_;
> > +	}
> > +
> > +private:
> > +	void readyRead(IPCUnixSocket *ipc)
> > +	{
> > +		IPCUnixSocket::Payload message, response;
> > +		int ret;
> > +
> > +		if (ipc->receive(&message)) {
> > +			cerr << "Receive message failed" << endl;
> > +			return;
> > +		}
> > +
> > +		const uint8_t cmd = message.data[0];
> > +		cout << "Slave received command " << static_cast<unsigned int>(cmd) << endl;
> > +
> > +		switch (cmd) {
> > +		case CMD_CLOSE:
> > +			stop(0);
> > +			break;
> > +
> > +		case CMD_REVERSE: {
> > +			response.data = message.data;
> > +			std::reverse(response.data.begin() + 1, response.data.end());
> > +
> > +			ret = ipc_.send(response);
> > +			if (ret < 0) {
> > +				cerr << "Reverse fail" << endl;
> > +				stop(ret);
> > +			}
> > +			break;
> > +		}
> > +
> > +		case CMD_LEN_CALC: {
> > +			int size = 0;
> > +			for (int fd : message.fds)
> > +				size += calculateLength(fd);
> > +
> > +			response.data.resize(1 + sizeof(size));
> > +			response.data[0] = cmd;
> > +			memcpy(response.data.data() + 1, &size, sizeof(size));
> > +
> > +			ret = ipc_.send(response);
> > +			if (ret < 0) {
> > +				cerr << "Calc fail" << endl;
> > +				stop(ret);
> > +			}
> > +			break;
> > +		}
> > +
> > +		case CMD_LEN_CMP: {
> > +			int size = 0;
> > +			for (int fd : message.fds)
> > +				size += calculateLength(fd);
> > +
> > +			int cmp;
> > +			memcpy(&cmp, message.data.data() + 1, sizeof(cmp));
> > +
> > +			if (cmp != size) {
> > +				cerr << "Compare fail" << endl;
> > +				stop(-ERANGE);
> > +			}
> > +			break;
> > +		}
> > +
> > +		default:
> > +			cerr << "Unknown command " << cmd << endl;
> > +			stop(-EINVAL);
> > +			break;
> > +		}
> > +	}
> > +
> > +	void stop(int code)
> > +	{
> > +		exitCode_ = code;
> > +		exit_ = true;
> > +	}
> > +
> > +	IPCUnixSocket ipc_;
> > +	EventDispatcher *dispatcher_;
> > +	int exitCode_;
> > +	bool exit_;
> > +};
> > +
> > +class UnixSocketTest : public Test
> > +{
> > +protected:
> > +	int slaveStart(int fd)
> > +	{
> > +		pid_ = fork();
> > +
> > +		if (pid_ == -1)
> > +			return TestFail;
> > +
> > +		if (!pid_) {
> > +			std::string arg = std::to_string(fd);
> > +			execl("/proc/self/exe", "/proc/self/exe",
> > +			      arg.c_str(), nullptr);
> > +
> > +			/* Only get here if exec fails. */
> > +			exit(TestFail);
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	int slaveStop()
> > +	{
> > +		int status;
> > +
> > +		if (pid_ < 0)
> > +			return TestFail;
> > +
> > +		if (waitpid(pid_, &status, 0) < 0)
> > +			return TestFail;
> > +
> > +		if (!WIFEXITED(status) || WEXITSTATUS(status))
> > +			return TestFail;
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	int testReverse()
> > +	{
> > +		IPCUnixSocket::Payload message, response;
> > +		int ret;
> > +
> > +		message.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };
> > +
> > +		ret = call(message, &response);
> > +		if (ret)
> > +			return ret;
> > +
> > +		std::reverse(response.data.begin() + 1, response.data.end());
> > +		if (message.data != response.data)
> > +			return TestFail;
> > +
> > +		return 0;
> > +	}
> > +
> > +	int testCalc()
> > +	{
> > +		IPCUnixSocket::Payload message, response;
> > +		int sizeOut, sizeIn, ret;
> > +
> > +		sizeOut = prepareFDs(&message, 2);
> > +		if (sizeOut < 0)
> > +			return sizeOut;
> > +
> > +		message.data.push_back(CMD_LEN_CALC);
> > +
> > +		ret = call(message, &response);
> > +		if (ret)
> > +			return ret;
> > +
> > +		memcpy(&sizeIn, response.data.data() + 1, sizeof(sizeIn));
> > +		if (sizeOut != sizeIn)
> > +			return TestFail;
> > +
> > +		return 0;
> > +	}
> > +
> > +	int testCmp()
> > +	{
> > +		IPCUnixSocket::Payload message;
> > +		int size;
> > +
> > +		size = prepareFDs(&message, 7);
> > +		if (size < 0)
> > +			return size;
> > +
> > +		message.data.resize(1 + sizeof(size));
> > +		message.data[0] = CMD_LEN_CMP;
> > +		memcpy(message.data.data() + 1, &size, sizeof(size));
> > +
> > +		if (ipc_.send(message))
> > +			return TestFail;
> > +
> > +		return 0;
> > +	}
> > +
> > +	int init()
> > +	{
> > +		callResponse_ = nullptr;
> > +		return 0;
> > +	}
> > +
> > +	int run()
> > +	{
> > +		int slavefd = ipc_.create();
> > +		if (slavefd < 0)
> > +			return TestFail;
> > +
> > +		if (slaveStart(slavefd)) {
> > +			cerr << "Failed to start slave" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		ipc_.readyRead.connect(this, &UnixSocketTest::readyRead);
> > +
> > +		/* Test reversing a string, this test sending only data. */
> > +		if (testReverse()) {
> > +			cerr << "Reveres array test failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test offloading a calculation, this test sending only FDs. */
> > +		if (testCalc()) {
> > +			cerr << "Calc test failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test fire and forget, this tests sending data and FDs. */
> > +		if (testCmp()) {
> > +			cerr << "Cmp test failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Close slave connection. */
> > +		IPCUnixSocket::Payload close;
> > +		close.data.push_back(CMD_CLOSE);
> > +		if (ipc_.send(close)) {
> > +			cerr << "Closing IPC channel failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		ipc_.close();
> > +		if (slaveStop()) {
> > +			cerr << "Failed to stop slave" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +
> > +private:
> > +	int call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response)
> > +	{
> > +		Timer timeout;
> > +		int ret;
> > +
> > +		if (callResponse_)
> > +			return -EBUSY;
> 
> I think you can skip this as all return paths of this function set
> callResponse_ to nullptr.
> 
> > +
> > +		callDone_ = false;
> > +		callResponse_ = response;
> > +
> > +		ret = ipc_.send(message);
> > +		if (ret)
> > +			return ret;
> > +
> > +		timeout.start(200);
> > +		while (!callDone_) {
> > +			if (!timeout.isRunning()) {
> > +				cerr << "Call timeout!" << endl;
> > +				callResponse_ = nullptr;
> > +				return -ETIMEDOUT;
> > +			}
> > +
> > +			CameraManager::instance()->eventDispatcher()->processEvents();
> > +		}
> > +
> > +		callResponse_ = nullptr;
> > +
> > +		return 0;
> > +	}
> > +
> > +	void readyRead(IPCUnixSocket *ipc)
> > +	{
> > +		IPCUnixSocket::Payload message;
> 
> Unused variable. Didn't the compiler warn ?

Nope, does yours?

I have never received any help from g++ nor clang when it comes to 
warning about unused none basic data types, I'm starting to believe it's 
not possible due to this could have an effect as IPCUnixSocket::Payload 
constructor could do a lot of things which are intentional even tho the 
variable itself is not used.

In the past I been able to shake out a few unused basic data type 
variables from loops when comping with -O0, but that do not catch this 
variable for me.

> 
> > +
> > +		if (!callResponse_) {
> > +			cerr << "Read ready without expecting data, fail." << endl;
> > +			return;
> > +		}
> > +
> > +		if (ipc->receive(callResponse_)) {
> > +			cerr << "Receive message failed" << endl;
> > +			return;
> > +		}
> > +
> > +		const uint8_t cmd = callResponse_->data[0];
> > +		cout << "Master received command " << static_cast<unsigned int>(cmd) << endl;
> 
> I would drop this print to avoid outputting message when the test
> succeeds.
> 
> > +
> > +		callDone_ = true;
> > +	}
> > +
> > +	int prepareFDs(IPCUnixSocket::Payload *message, unsigned int num)
> > +	{
> > +		int fd = open("/proc/self/exe", O_RDONLY);
> > +		if (fd < 0)
> > +			return fd;
> > +
> > +		int size = 0;
> > +		for (unsigned int i = 0; i < num; i++) {
> > +			int clone = dup(fd);
> > +			if (clone < 0)
> > +				return clone;
> > +
> > +			size += calculateLength(clone);
> > +			message->fds.push_back(clone);
> > +		}
> 
> I still think it would be nicer to use mkstemp() to create temporary
> files, and test their content instead of just the size. You could then
> drop one of the two fd-related tests, and for instance create 2
> temporary files with data, send their fds, and receive back the fd of a
> new temporary file that contains the concatenation of the two.

I think this one is nicer :-)

I'm quiet confident that file IO works in the system, what I'm not so 
confident in is that the IPC mechanism is able to pass multiple FDs.  
With this construct we test passing 2 and 7 file descriptors and verify 
that the FDs are valid by the most simple operation on them. Adding a 
test which you describe would indeed test a similar thing but IMHO in a 
more complex way with no benefit to the IPC verification.

Also as soon as we start creating temporary files in a TC we also need 
to care more about cleanup logic. As we don't wish to leave waste after 
us, with this construct it's not an issue.

> 
> > +
> > +		close(fd);
> > +
> > +		return size;
> > +	}
> > +
> > +	pid_t pid_;
> > +	IPCUnixSocket ipc_;
> > +	bool callDone_;
> > +	IPCUnixSocket::Payload *callResponse_;
> > +};
> > +
> > +/*
> > + * Can't use TEST_REGISTER() as single binary needs to act as both proxy
> > + * master and slave.
> > + */
> > +int main(int argc, char **argv)
> > +{
> > +	if (argc == 2) {
> > +		int ipcfd = std::stoi(argv[1]);
> > +		UnixSocketTestSlave slave;
> > +		return slave.run(ipcfd);
> > +	}
> > +
> > +	return UnixSocketTest().execute();
> > +}
> > diff --git a/test/meson.build b/test/meson.build
> > index c36ac24796367501..3666f6b2385bd4ca 100644
> > --- a/test/meson.build
> > +++ b/test/meson.build
> > @@ -2,6 +2,7 @@ subdir('libtest')
> >  
> >  subdir('camera')
> >  subdir('ipa')
> > +subdir('ipc')
> >  subdir('media_device')
> >  subdir('pipeline')
> >  subdir('stream')
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Hi Niklas,

On Mon, Jul 01, 2019 at 11:27:26AM +0200, Niklas Söderlund wrote:
> On 2019-07-01 03:10:52 +0300, Laurent Pinchart wrote:
> > On Sun, Jun 30, 2019 at 06:25:14PM +0200, Niklas Söderlund wrote:
> >> Test that the IPC supports sending data and file descriptors over the
> >> IPC medium. To be able to execute the test two parts are needed, one
> >> to drive the test and act as the libcamera (master) and a one to act as
> >> the IPA (slave).
> >> 
> >> The master drives the testing posting requests to the slave to process
> >> and sometimes respond to. A few different tests are performed.
> >> 
> >> - Master sends an array to the slave which responds with a reversed copy
> >>   of the array. The master verifies that a reversed array is returned.
> >> 
> >> - Master sends a list of file descriptors and ask the slave to calculate
> >>   and respond with the sum of the size of the files. The master verifies
> >>   that the calculated size is correct.
> >> 
> >> - Master sends a pre-computed size and a list of file descriptors and
> >>   ask the slave to verify that the pre-computed size matches the sum of
> > 
> > s/ask/asks/
> > 
> >>   the size of the file descriptors.
> >> 
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> >> ---
> >>  test/ipc/meson.build    |  12 ++
> >>  test/ipc/unixsocket.cpp | 390 ++++++++++++++++++++++++++++++++++++++++
> >>  test/meson.build        |   1 +
> >>  3 files changed, 403 insertions(+)
> >>  create mode 100644 test/ipc/meson.build
> >>  create mode 100644 test/ipc/unixsocket.cpp
> >> 
> >> diff --git a/test/ipc/meson.build b/test/ipc/meson.build
> >> new file mode 100644
> >> index 0000000000000000..ca8375f35df91731
> >> --- /dev/null
> >> +++ b/test/ipc/meson.build
> >> @@ -0,0 +1,12 @@
> >> +ipc_tests = [
> >> +    [ 'unixsocket',  'unixsocket.cpp' ],
> >> +]
> >> +
> >> +foreach t : ipc_tests
> >> +    exe = executable(t[0], t[1],
> >> +                     dependencies : libcamera_dep,
> >> +                     link_with : test_libraries,
> >> +                     include_directories : test_includes_internal)
> >> +
> >> +    test(t[0], exe, suite : 'ipc', is_parallel : false)
> >> +endforeach
> >> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp
> >> new file mode 100644
> >> index 0000000000000000..f433102af1b6d2a3
> >> --- /dev/null
> >> +++ b/test/ipc/unixsocket.cpp
> >> @@ -0,0 +1,390 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * Copyright (C) 2019, Google Inc.
> >> + *
> >> + * unixsocket.cpp - Unix socket IPC test
> >> + */
> >> +
> >> +#include <algorithm>
> >> +#include <atomic>
> >> +#include <fcntl.h>
> >> +#include <iostream>
> >> +#include <string.h>
> >> +#include <sys/wait.h>
> >> +#include <unistd.h>
> >> +
> >> +#include <libcamera/camera_manager.h>
> >> +#include <libcamera/event_dispatcher.h>
> >> +#include <libcamera/timer.h>
> >> +
> >> +#include "ipc_unixsocket.h"
> >> +#include "test.h"
> >> +
> >> +#define CMD_CLOSE	0
> >> +#define CMD_REVERSE	1
> >> +#define CMD_LEN_CALC	2
> >> +#define CMD_LEN_CMP	3
> >> +
> >> +using namespace std;
> >> +using namespace libcamera;
> >> +
> >> +int calculateLength(int fd)
> >> +{
> >> +	lseek(fd, 0, 0);
> >> +	int size = lseek(fd, 0, SEEK_END);
> >> +	lseek(fd, 0, 0);
> >> +
> >> +	return size;
> >> +}
> >> +
> >> +class UnixSocketTestSlave
> >> +{
> >> +public:
> >> +	UnixSocketTestSlave()
> >> +		: exitCode_(EXIT_FAILURE), exit_(false)
> >> +	{
> >> +		dispatcher_ = CameraManager::instance()->eventDispatcher();
> >> +		ipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead);
> >> +	}
> >> +
> >> +	int run(int fd)
> >> +	{
> >> +		if (ipc_.bind(fd)) {
> >> +			cerr << "Failed to connect to IPC channel" << endl;
> >> +			return EXIT_FAILURE;
> >> +		}
> >> +
> >> +		while (!exit_)
> >> +			dispatcher_->processEvents();
> >> +
> >> +		ipc_.close();
> >> +
> >> +		return exitCode_;
> >> +	}
> >> +
> >> +private:
> >> +	void readyRead(IPCUnixSocket *ipc)
> >> +	{
> >> +		IPCUnixSocket::Payload message, response;
> >> +		int ret;
> >> +
> >> +		if (ipc->receive(&message)) {
> >> +			cerr << "Receive message failed" << endl;
> >> +			return;
> >> +		}
> >> +
> >> +		const uint8_t cmd = message.data[0];
> >> +		cout << "Slave received command " << static_cast<unsigned int>(cmd) << endl;
> >> +
> >> +		switch (cmd) {
> >> +		case CMD_CLOSE:
> >> +			stop(0);
> >> +			break;
> >> +
> >> +		case CMD_REVERSE: {
> >> +			response.data = message.data;
> >> +			std::reverse(response.data.begin() + 1, response.data.end());
> >> +
> >> +			ret = ipc_.send(response);
> >> +			if (ret < 0) {
> >> +				cerr << "Reverse fail" << endl;
> >> +				stop(ret);
> >> +			}
> >> +			break;
> >> +		}
> >> +
> >> +		case CMD_LEN_CALC: {
> >> +			int size = 0;
> >> +			for (int fd : message.fds)
> >> +				size += calculateLength(fd);
> >> +
> >> +			response.data.resize(1 + sizeof(size));
> >> +			response.data[0] = cmd;
> >> +			memcpy(response.data.data() + 1, &size, sizeof(size));
> >> +
> >> +			ret = ipc_.send(response);
> >> +			if (ret < 0) {
> >> +				cerr << "Calc fail" << endl;
> >> +				stop(ret);
> >> +			}
> >> +			break;
> >> +		}
> >> +
> >> +		case CMD_LEN_CMP: {
> >> +			int size = 0;
> >> +			for (int fd : message.fds)
> >> +				size += calculateLength(fd);
> >> +
> >> +			int cmp;
> >> +			memcpy(&cmp, message.data.data() + 1, sizeof(cmp));
> >> +
> >> +			if (cmp != size) {
> >> +				cerr << "Compare fail" << endl;
> >> +				stop(-ERANGE);
> >> +			}
> >> +			break;
> >> +		}
> >> +
> >> +		default:
> >> +			cerr << "Unknown command " << cmd << endl;
> >> +			stop(-EINVAL);
> >> +			break;
> >> +		}
> >> +	}
> >> +
> >> +	void stop(int code)
> >> +	{
> >> +		exitCode_ = code;
> >> +		exit_ = true;
> >> +	}
> >> +
> >> +	IPCUnixSocket ipc_;
> >> +	EventDispatcher *dispatcher_;
> >> +	int exitCode_;
> >> +	bool exit_;
> >> +};
> >> +
> >> +class UnixSocketTest : public Test
> >> +{
> >> +protected:
> >> +	int slaveStart(int fd)
> >> +	{
> >> +		pid_ = fork();
> >> +
> >> +		if (pid_ == -1)
> >> +			return TestFail;
> >> +
> >> +		if (!pid_) {
> >> +			std::string arg = std::to_string(fd);
> >> +			execl("/proc/self/exe", "/proc/self/exe",
> >> +			      arg.c_str(), nullptr);
> >> +
> >> +			/* Only get here if exec fails. */
> >> +			exit(TestFail);
> >> +		}
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +	int slaveStop()
> >> +	{
> >> +		int status;
> >> +
> >> +		if (pid_ < 0)
> >> +			return TestFail;
> >> +
> >> +		if (waitpid(pid_, &status, 0) < 0)
> >> +			return TestFail;
> >> +
> >> +		if (!WIFEXITED(status) || WEXITSTATUS(status))
> >> +			return TestFail;
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +	int testReverse()
> >> +	{
> >> +		IPCUnixSocket::Payload message, response;
> >> +		int ret;
> >> +
> >> +		message.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };
> >> +
> >> +		ret = call(message, &response);
> >> +		if (ret)
> >> +			return ret;
> >> +
> >> +		std::reverse(response.data.begin() + 1, response.data.end());
> >> +		if (message.data != response.data)
> >> +			return TestFail;
> >> +
> >> +		return 0;
> >> +	}
> >> +
> >> +	int testCalc()
> >> +	{
> >> +		IPCUnixSocket::Payload message, response;
> >> +		int sizeOut, sizeIn, ret;
> >> +
> >> +		sizeOut = prepareFDs(&message, 2);
> >> +		if (sizeOut < 0)
> >> +			return sizeOut;
> >> +
> >> +		message.data.push_back(CMD_LEN_CALC);
> >> +
> >> +		ret = call(message, &response);
> >> +		if (ret)
> >> +			return ret;
> >> +
> >> +		memcpy(&sizeIn, response.data.data() + 1, sizeof(sizeIn));
> >> +		if (sizeOut != sizeIn)
> >> +			return TestFail;
> >> +
> >> +		return 0;
> >> +	}
> >> +
> >> +	int testCmp()
> >> +	{
> >> +		IPCUnixSocket::Payload message;
> >> +		int size;
> >> +
> >> +		size = prepareFDs(&message, 7);
> >> +		if (size < 0)
> >> +			return size;
> >> +
> >> +		message.data.resize(1 + sizeof(size));
> >> +		message.data[0] = CMD_LEN_CMP;
> >> +		memcpy(message.data.data() + 1, &size, sizeof(size));
> >> +
> >> +		if (ipc_.send(message))
> >> +			return TestFail;
> >> +
> >> +		return 0;
> >> +	}
> >> +
> >> +	int init()
> >> +	{
> >> +		callResponse_ = nullptr;
> >> +		return 0;
> >> +	}
> >> +
> >> +	int run()
> >> +	{
> >> +		int slavefd = ipc_.create();
> >> +		if (slavefd < 0)
> >> +			return TestFail;
> >> +
> >> +		if (slaveStart(slavefd)) {
> >> +			cerr << "Failed to start slave" << endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		ipc_.readyRead.connect(this, &UnixSocketTest::readyRead);
> >> +
> >> +		/* Test reversing a string, this test sending only data. */
> >> +		if (testReverse()) {
> >> +			cerr << "Reveres array test failed" << endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		/* Test offloading a calculation, this test sending only FDs. */
> >> +		if (testCalc()) {
> >> +			cerr << "Calc test failed" << endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		/* Test fire and forget, this tests sending data and FDs. */
> >> +		if (testCmp()) {
> >> +			cerr << "Cmp test failed" << endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		/* Close slave connection. */
> >> +		IPCUnixSocket::Payload close;
> >> +		close.data.push_back(CMD_CLOSE);
> >> +		if (ipc_.send(close)) {
> >> +			cerr << "Closing IPC channel failed" << endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		ipc_.close();
> >> +		if (slaveStop()) {
> >> +			cerr << "Failed to stop slave" << endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +private:
> >> +	int call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response)
> >> +	{
> >> +		Timer timeout;
> >> +		int ret;
> >> +
> >> +		if (callResponse_)
> >> +			return -EBUSY;
> > 
> > I think you can skip this as all return paths of this function set
> > callResponse_ to nullptr.
> > 
> >> +
> >> +		callDone_ = false;
> >> +		callResponse_ = response;
> >> +
> >> +		ret = ipc_.send(message);
> >> +		if (ret)
> >> +			return ret;
> >> +
> >> +		timeout.start(200);
> >> +		while (!callDone_) {
> >> +			if (!timeout.isRunning()) {
> >> +				cerr << "Call timeout!" << endl;
> >> +				callResponse_ = nullptr;
> >> +				return -ETIMEDOUT;
> >> +			}
> >> +
> >> +			CameraManager::instance()->eventDispatcher()->processEvents();
> >> +		}
> >> +
> >> +		callResponse_ = nullptr;
> >> +
> >> +		return 0;
> >> +	}
> >> +
> >> +	void readyRead(IPCUnixSocket *ipc)
> >> +	{
> >> +		IPCUnixSocket::Payload message;
> > 
> > Unused variable. Didn't the compiler warn ?
> 
> Nope, does yours?

It usually does, I haven't checked here.

> I have never received any help from g++ nor clang when it comes to 
> warning about unused none basic data types, I'm starting to believe it's 
> not possible due to this could have an effect as IPCUnixSocket::Payload 
> constructor could do a lot of things which are intentional even tho the 
> variable itself is not used.
> 
> In the past I been able to shake out a few unused basic data type 
> variables from loops when comping with -O0, but that do not catch this 
> variable for me.
> 
> >> +
> >> +		if (!callResponse_) {
> >> +			cerr << "Read ready without expecting data, fail." << endl;
> >> +			return;
> >> +		}
> >> +
> >> +		if (ipc->receive(callResponse_)) {
> >> +			cerr << "Receive message failed" << endl;
> >> +			return;
> >> +		}
> >> +
> >> +		const uint8_t cmd = callResponse_->data[0];
> >> +		cout << "Master received command " << static_cast<unsigned int>(cmd) << endl;
> > 
> > I would drop this print to avoid outputting message when the test
> > succeeds.
> > 
> >> +
> >> +		callDone_ = true;
> >> +	}
> >> +
> >> +	int prepareFDs(IPCUnixSocket::Payload *message, unsigned int num)
> >> +	{
> >> +		int fd = open("/proc/self/exe", O_RDONLY);
> >> +		if (fd < 0)
> >> +			return fd;
> >> +
> >> +		int size = 0;
> >> +		for (unsigned int i = 0; i < num; i++) {
> >> +			int clone = dup(fd);
> >> +			if (clone < 0)
> >> +				return clone;
> >> +
> >> +			size += calculateLength(clone);
> >> +			message->fds.push_back(clone);
> >> +		}
> > 
> > I still think it would be nicer to use mkstemp() to create temporary
> > files, and test their content instead of just the size. You could then
> > drop one of the two fd-related tests, and for instance create 2
> > temporary files with data, send their fds, and receive back the fd of a
> > new temporary file that contains the concatenation of the two.
> 
> I think this one is nicer :-)
> 
> I'm quiet confident that file IO works in the system, what I'm not so 
> confident in is that the IPC mechanism is able to pass multiple FDs.  
> With this construct we test passing 2 and 7 file descriptors and verify 
> that the FDs are valid by the most simple operation on them. Adding a 
> test which you describe would indeed test a similar thing but IMHO in a 
> more complex way with no benefit to the IPC verification.

My concern here is that by passing multiple fds refering to the same
file, you may indeed be testing passing multiple fds, but you can't
verify the ordering :-S

> Also as soon as we start creating temporary files in a TC we also need 
> to care more about cleanup logic. As we don't wish to leave waste after 
> us, with this construct it's not an issue.

I was thinking you could unlink the file as soon as it gets created, but
another option would be to use shm_open() to avoid creating files on
disk.

> >> +
> >> +		close(fd);
> >> +
> >> +		return size;
> >> +	}
> >> +
> >> +	pid_t pid_;
> >> +	IPCUnixSocket ipc_;
> >> +	bool callDone_;
> >> +	IPCUnixSocket::Payload *callResponse_;
> >> +};
> >> +
> >> +/*
> >> + * Can't use TEST_REGISTER() as single binary needs to act as both proxy
> >> + * master and slave.
> >> + */
> >> +int main(int argc, char **argv)
> >> +{
> >> +	if (argc == 2) {
> >> +		int ipcfd = std::stoi(argv[1]);
> >> +		UnixSocketTestSlave slave;
> >> +		return slave.run(ipcfd);
> >> +	}
> >> +
> >> +	return UnixSocketTest().execute();
> >> +}
> >> diff --git a/test/meson.build b/test/meson.build
> >> index c36ac24796367501..3666f6b2385bd4ca 100644
> >> --- a/test/meson.build
> >> +++ b/test/meson.build
> >> @@ -2,6 +2,7 @@ subdir('libtest')
> >>  
> >>  subdir('camera')
> >>  subdir('ipa')
> >> +subdir('ipc')
> >>  subdir('media_device')
> >>  subdir('pipeline')
> >>  subdir('stream')

Hi Laurent,

Thanks for your feedback.

On 2019-07-01 13:15:29 +0300, Laurent Pinchart wrote:
> > >> +	int prepareFDs(IPCUnixSocket::Payload *message, unsigned 
> > >> int num)
> > >> +	{
> > >> +		int fd = open("/proc/self/exe", O_RDONLY);
> > >> +		if (fd < 0)
> > >> +			return fd;
> > >> +
> > >> +		int size = 0;
> > >> +		for (unsigned int i = 0; i < num; i++) {
> > >> +			int clone = dup(fd);
> > >> +			if (clone < 0)
> > >> +				return clone;
> > >> +
> > >> +			size += calculateLength(clone);
> > >> +			message->fds.push_back(clone);
> > >> +		}
> > > 
> > > I still think it would be nicer to use mkstemp() to create temporary
> > > files, and test their content instead of just the size. You could then
> > > drop one of the two fd-related tests, and for instance create 2
> > > temporary files with data, send their fds, and receive back the fd of a
> > > new temporary file that contains the concatenation of the two.
> > 
> > I think this one is nicer :-)
> > 
> > I'm quiet confident that file IO works in the system, what I'm not so 
> > confident in is that the IPC mechanism is able to pass multiple FDs.  
> > With this construct we test passing 2 and 7 file descriptors and verify 
> > that the FDs are valid by the most simple operation on them. Adding a 
> > test which you describe would indeed test a similar thing but IMHO in a 
> > more complex way with no benefit to the IPC verification.
> 
> My concern here is that by passing multiple fds refering to the same
> file, you may indeed be testing passing multiple fds, but you can't
> verify the ordering :-S

This is a reason I can agree with. I will add a test to verify ordering 
of fds, but I will keep the use of dup() here as it tests transferring 
may fds.

> 
> > Also as soon as we start creating temporary files in a TC we also need 
> > to care more about cleanup logic. As we don't wish to leave waste after 
> > us, with this construct it's not an issue.
> 
> I was thinking you could unlink the file as soon as it gets created, but
> another option would be to use shm_open() to avoid creating files on
> disk.

Nice tip, I will try to make use of shm_open() in the next version for 
the additional test.