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

[Hirokazu Honda]

Hi Laurent, thank you for the patch.

On Mon, Nov 29, 2021 at 8:58 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> 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()
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 at 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
>