[libcamera-devel,v3,05/17] test: Add UniqueFD test
diff mbox series

Message ID 20211128235752.10836-6-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Introduce UniqueFD
Related show

Commit Message

Laurent Pinchart Nov. 28, 2021, 11:57 p.m. UTC 
Add a unit test to exercise the API of the UniqueFD class.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/meson.build   |   1 +
 test/unique-fd.cpp | 220 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 221 insertions(+)
 create mode 100644 test/unique-fd.cpp

Comments

Hirokazu Honda Nov. 29, 2021, 1:36 p.m. UTC | #1 
Hi Laurent, thank you for the patch.

On Mon, Nov 29, 2021 at 8:58 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Add a unit test to exercise the API of the UniqueFD class.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@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()
init() override
> +       {
> +               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()
run() override

> +       {
> +               /* Test creating empty UniqueFD. */
> +               UniqueFD fd;
> +
> +               if (fd.isValid() || fd.get() != -1) {

I would not check the initialization value is -1.
The point is fd.isValid() is false with a default constructor and the
default value is not matter IMO.
If you would like, then I would declare the static constexpr value
like kInvalidFDValue and compare with it.

> +                       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_)) {
> +                       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) {

ditto.

> +                       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;
> +               }

ditto.

> +
> +               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;
> +               }
> +
> +               return TestPass;
> +       }
> +
> +       void cleanup()

cleanup() override

With minor nits,

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> +       {
> +               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
>
Jacopo Mondi Nov. 29, 2021, 3:07 p.m. UTC | #2 
Hi Laurent

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@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 ?

> +	{
> +		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 &&) ?)

> +			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@jmondi.org>

Thanks
  j

> +
> +		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
>
Laurent Pinchart Nov. 29, 2021, 5:31 p.m. UTC | #3 
Hi Hiro,

On Mon, Nov 29, 2021 at 10:36:17PM +0900, Hirokazu Honda wrote:
> On Mon, Nov 29, 2021 at 8:58 AM Laurent Pinchart wrote:
> >
> > Add a unit test to exercise the API of the UniqueFD class.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@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()
>
> init() override

Oops.

> > +       {
> > +               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()
>
> run() override

Re-oops :-)

> > +       {
> > +               /* Test creating empty UniqueFD. */
> > +               UniqueFD fd;
> > +
> > +               if (fd.isValid() || fd.get() != -1) {
> 
> I would not check the initialization value is -1.
> The point is fd.isValid() is false with a default constructor and the
> default value is not matter IMO.
> If you would like, then I would declare the static constexpr value
> like kInvalidFDValue and compare with it.

It depends on the API contract I think. The documentation explicitly
states that get() returns -1 if no fd is owned, so that's why I'm
checking it. This matches std::unique_ptr<>::get() that returns nullptr
if no object is owned. The alternative is to make get() an undefined
behaviour if no fd is owned, and drop the checks here, but I don't think
that would be a good idea. If get() is explicitly defined as returning
-1 when no fd is owned, then I think that should be tested here.

> > +                       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_)) {
> > +                       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) {
> 
> ditto.
> 
> > +                       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;
> > +               }
> 
> ditto.
> 
> > +
> > +               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;
> > +               }
> > +
> > +               return TestPass;
> > +       }
> > +
> > +       void cleanup()
> 
> cleanup() override
> 
> With minor nits,
> 
> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
>
> > +       {
> > +               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)
Laurent Pinchart Nov. 29, 2021, 5:41 p.m. UTC | #4 
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@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@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)
Hirokazu Honda Nov. 30, 2021, 1:13 a.m. UTC | #5 
Hi Laurent,

On Tue, Nov 30, 2021 at 2:32 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Hiro,
>
> On Mon, Nov 29, 2021 at 10:36:17PM +0900, Hirokazu Honda wrote:
> > On Mon, Nov 29, 2021 at 8:58 AM Laurent Pinchart wrote:
> > >
> > > Add a unit test to exercise the API of the UniqueFD class.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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()
> >
> > init() override
>
> Oops.
>
> > > +       {
> > > +               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()
> >
> > run() override
>
> Re-oops :-)
>
> > > +       {
> > > +               /* Test creating empty UniqueFD. */
> > > +               UniqueFD fd;
> > > +
> > > +               if (fd.isValid() || fd.get() != -1) {
> >
> > I would not check the initialization value is -1.
> > The point is fd.isValid() is false with a default constructor and the
> > default value is not matter IMO.
> > If you would like, then I would declare the static constexpr value
> > like kInvalidFDValue and compare with it.
>
> It depends on the API contract I think. The documentation explicitly
> states that get() returns -1 if no fd is owned, so that's why I'm
> checking it. This matches std::unique_ptr<>::get() that returns nullptr
> if no object is owned. The alternative is to make get() an undefined
> behaviour if no fd is owned, and drop the checks here, but I don't think
> that would be a good idea. If get() is explicitly defined as returning
> -1 when no fd is owned, then I think that should be tested here.
>

Ack. I am fine to keep the code as-is.

-Hiro
> > > +                       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_)) {
> > > +                       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) {
> >
> > ditto.
> >
> > > +                       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;
> > > +               }
> >
> > ditto.
> >
> > > +
> > > +               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;
> > > +               }
> > > +
> > > +               return TestPass;
> > > +       }
> > > +
> > > +       void cleanup()
> >
> > cleanup() override
> >
> > With minor nits,
> >
> > Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> >
> > > +       {
> > > +               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

Patch
diff mbox series 

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()
+	{
+		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_)) {
+			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;
+		}
+
+		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)