[libcamera-devel] [PATCH v3 05/17] test: Add UniqueFD test

[Laurent Pinchart]

Hi Jacopo,

On Mon, Nov 29, 2021 at 04:07:04PM +0100, Jacopo Mondi wrote:
> On Mon, Nov 29, 2021 at 01:57:40AM +0200, Laurent Pinchart wrote:
> > Add a unit test to exercise the API of the UniqueFD class.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  test/meson.build   |   1 +
> >  test/unique-fd.cpp | 220 +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 221 insertions(+)
> >  create mode 100644 test/unique-fd.cpp
> >
> > diff --git a/test/meson.build b/test/meson.build
> > index d0466f17d7b6..42dfbc1f8ee9 100644
> > --- a/test/meson.build
> > +++ b/test/meson.build
> > @@ -53,6 +53,7 @@ internal_tests = [
> >      ['threads',                         'threads.cpp'],
> >      ['timer',                           'timer.cpp'],
> >      ['timer-thread',                    'timer-thread.cpp'],
> > +    ['unique-fd',                       'unique-fd.cpp'],
> >      ['utils',                           'utils.cpp'],
> >  ]
> >
> > diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp
> > new file mode 100644
> > index 000000000000..92ca3f328811
> > --- /dev/null
> > +++ b/test/unique-fd.cpp
> > @@ -0,0 +1,220 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2021, Google Inc.
> > + *
> > + * unique-fd.cpp - UniqueFD test
> > + */
> > +
> > +#include <fcntl.h>
> > +#include <iostream>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +
> > +#include <libcamera/base/unique_fd.h>
> > +#include <libcamera/base/utils.h>
> > +
> > +#include "test.h"
> > +
> > +using namespace libcamera;
> > +using namespace std;
> > +
> > +class UniqueFDTest : public Test
> > +{
> > +protected:
> > +	int init()
> 
> override ?

Absolutely.

> > +	{
> > +		return createFd();
> > +	}
> > +
> > +	int createFd()
> > +	{
> > +		fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);
> > +		if (fd_ < 0)
> > +			return TestFail;
> > +
> > +		/* Cache inode number of temp file. */
> > +		struct stat s;
> > +		if (fstat(fd_, &s))
> > +			return TestFail;
> > +
> > +		inodeNr_ = s.st_ino;
> > +
> > +		return 0;
> > +	}
> > +
> > +	int run()
> > +	{
> > +		/* Test creating empty UniqueFD. */
> > +		UniqueFD fd;
> > +
> > +		if (fd.isValid() || fd.get() != -1) {
> > +			std::cout << "Failed fd check (default constructor)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test creating UniqueFD from numerical file descriptor. */
> > +		UniqueFD fd2(fd_);
> > +		if (!fd2.isValid() || fd2.get() != fd_) {
> > +			std::cout << "Failed fd check (fd constructor)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (!isValidFd(fd_)) {
> 
> Ups, I missed that in the review of the patch that introduced
> UniqueFD, but shouldn't constructing by && reset the parameter to -1 ?
> (This can also be read as: Shouldn't we have UniqueFD(int &&) ?)

Let's discuss it in patch 04/17.

> > +			std::cout << "Failed fd validity (fd constructor)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test move constructor. */
> > +		UniqueFD fd3(std::move(fd2));
> > +		if (!fd3.isValid() || fd3.get() != fd_) {
> > +			std::cout << "Failed fd check (move constructor)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (fd2.isValid() || fd2.get() != -1) {
> > +			std::cout << "Failed moved fd check (move constructor)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (!isValidFd(fd_)) {
> > +			std::cout << "Failed fd validity (move constructor)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test move assignment operator. */
> > +		fd = std::move(fd3);
> > +		if (!fd.isValid() || fd.get() != fd_) {
> > +			std::cout << "Failed fd check (move assignment)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (fd3.isValid() || fd3.get() != -1) {
> > +			std::cout << "Failed moved fd check (move assignment)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (!isValidFd(fd_)) {
> > +			std::cout << "Failed fd validity (move assignment)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test swapping. */
> > +		fd2.swap(fd);
> > +		if (!fd2.isValid() || fd2.get() != fd_) {
> > +			std::cout << "Failed fd check (swap)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (fd.isValid() || fd.get() != -1) {
> > +			std::cout << "Failed swapped fd check (swap)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (!isValidFd(fd_)) {
> > +			std::cout << "Failed fd validity (swap)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test release. */
> > +		int numFd = fd2.release();
> > +		if (fd2.isValid() || fd2.get() != -1) {
> > +			std::cout << "Failed fd check (release)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (numFd != fd_) {
> > +			std::cout << "Failed released fd check (release)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (!isValidFd(fd_)) {
> > +			std::cout << "Failed fd validity (release)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test reset assignment. */
> > +		fd.reset(numFd);
> > +		if (!fd.isValid() || fd.get() != fd_) {
> > +			std::cout << "Failed fd check (reset assignment)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (!isValidFd(fd_)) {
> > +			std::cout << "Failed fd validity (reset assignment)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test reset destruction. */
> > +		fd.reset();
> > +		if (fd.isValid() || fd.get() != -1) {
> > +			std::cout << "Failed fd check (reset destruction)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (isValidFd(fd_)) {
> > +			std::cout << "Failed fd validity (reset destruction)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test destruction. */
> > +		if (createFd() == TestFail) {
> > +			std::cout << "Failed to recreate test fd"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		{
> > +			UniqueFD fd4(fd_);
> > +		}
> > +
> > +		if (isValidFd(fd_)) {
> > +			std::cout << "Failed fd validity (destruction)"
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> 
> nice!
> 
> For the test
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	void cleanup()
> > +	{
> > +		if (fd_ > 0)
> > +			close(fd_);
> > +	}
> > +
> > +private:
> > +	bool isValidFd(int fd)
> > +	{
> > +		struct stat s;
> > +		if (fstat(fd, &s))
> > +			return false;
> > +
> > +		/* Check that inode number matches cached temp file. */
> > +		return s.st_ino == inodeNr_;
> > +	}
> > +
> > +	int fd_;
> > +	ino_t inodeNr_;
> > +};
> > +
> > +TEST_REGISTER(UniqueFDTest)

-- 
Regards,

Laurent Pinchart