[libcamera-devel,v2,3/3] test: file_descriptor: Add test

Message ID 20191217015106.1175515-4-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: Add FileDescriptor to help pass numerical fds around
Related show

Commit Message

Niklas Söderlund Dec. 17, 2019, 1:51 a.m. UTC
Add a test which exercises the whole FileDescriptor interface.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/file_descriptor.cpp | 165 +++++++++++++++++++++++++++++++++++++++
 test/meson.build         |   1 +
 2 files changed, 166 insertions(+)
 create mode 100644 test/file_descriptor.cpp

Comments

Laurent Pinchart Dec. 17, 2019, 2:10 a.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Tue, Dec 17, 2019 at 02:51:06AM +0100, Niklas Söderlund wrote:
> Add a test which exercises the whole FileDescriptor interface.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  test/file_descriptor.cpp | 165 +++++++++++++++++++++++++++++++++++++++
>  test/meson.build         |   1 +
>  2 files changed, 166 insertions(+)
>  create mode 100644 test/file_descriptor.cpp
> 
> diff --git a/test/file_descriptor.cpp b/test/file_descriptor.cpp
> new file mode 100644
> index 0000000000000000..177d8bcf16591185
> --- /dev/null
> +++ b/test/file_descriptor.cpp
> @@ -0,0 +1,165 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * file_descriptor.cpp - FileDescriptor test
> + */
> +
> +#include <fcntl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include <libcamera/file_descriptor.h>
> +
> +#include "test.h"
> +
> +using namespace libcamera;
> +using namespace std;
> +
> +class FileDescriptorTest : public Test
> +{
> +protected:
> +	int init()
> +	{
> +		desc1_ = nullptr;
> +		desc2_ = nullptr;
> +
> +		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 FileDescriptor from numerical file descriptor. */
> +		desc1_ = new FileDescriptor(fd_);
> +		if (desc1_->fd() == fd_)
> +			return TestFail;
> +
> +		if (!isValidFd(fd_) || !isValidFd(desc1_->fd()))
> +			return TestFail;
> +
> +		int fd = desc1_->fd();
> +
> +		delete desc1_;
> +		desc1_ = nullptr;
> +
> +		if (!isValidFd(fd_) || isValidFd(fd))
> +			return TestFail;
> +
> +		/* Test creating FileDescriptor from other FileDescriptor. */
> +		desc1_ = new FileDescriptor(fd_);
> +		desc2_ = new FileDescriptor(*desc1_);
> +
> +		if (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() == desc2_->fd())
> +			return TestFail;
> +
> +		if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))
> +			return TestFail;
> +
> +		delete desc1_;
> +		desc1_ = nullptr;
> +
> +		if (!isValidFd(desc2_->fd()))
> +			return TestFail;
> +
> +		delete desc2_;
> +		desc2_ = nullptr;
> +
> +		/* Test creating FileDescriptor by taking over other FileDescriptor. */
> +		desc1_ = new FileDescriptor(fd_);
> +		fd = desc1_->fd();
> +		desc2_ = new FileDescriptor(std::move(*desc1_));
> +
> +		if (desc1_->fd() != -1 || desc2_->fd() != fd)
> +			return TestFail;
> +
> +		if (!isValidFd(desc2_->fd()))
> +			return TestFail;
> +
> +		delete desc1_;
> +		desc1_ = nullptr;
> +		delete desc2_;
> +		desc2_ = nullptr;
> +
> +		/* Test creating FileDescriptor by copy assignment. */
> +		desc1_ = new FileDescriptor();
> +		desc2_ = new FileDescriptor(fd_);
> +
> +		if (desc1_->fd() != -1)
> +			return TestFail;
> +
> +		fd = desc2_->fd();
> +		*desc1_ = *desc2_;
> +
> +		if (desc1_->fd() == fd || desc2_->fd() != fd)
> +			return TestFail;
> +
> +		if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))
> +			return TestFail;
> +
> +		delete desc1_;
> +		desc1_ = nullptr;
> +		delete desc2_;
> +		desc2_ = nullptr;
> +
> +		if (!isValidFd(fd_))
> +			return TestFail;

You can drop this check.

> +
> +		/* Test creating FileDescriptor by move assignment. */
> +		desc1_ = new FileDescriptor();
> +		desc2_ = new FileDescriptor(fd_);
> +
> +		fd = desc2_->fd();
> +		*desc1_ = std::move(*desc2_);
> +
> +		if (desc1_->fd() != fd || desc2_->fd() != -1)
> +			return TestFail;
> +
> +		if (!isValidFd(desc1_->fd()))
> +			return TestFail;
> +
> +		delete desc1_;
> +		desc1_ = nullptr;
> +		delete desc2_;
> +		desc2_ = nullptr;
> +
> +		return TestPass;
> +	}
> +
> +	void cleanup()
> +	{
> +		delete desc2_;
> +		delete desc1_;
> +
> +		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_;
> +	FileDescriptor *desc1_, *desc2_;
> +};
> +
> +TEST_REGISTER(FileDescriptorTest)
> diff --git a/test/meson.build b/test/meson.build
> index 1bb2161dc05a7b1d..9dc392df30efd956 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -25,6 +25,7 @@ internal_tests = [
>      ['event',                           'event.cpp'],
>      ['event-dispatcher',                'event-dispatcher.cpp'],
>      ['event-thread',                    'event-thread.cpp'],
> +    ['file_descriptor',                 'file_descriptor.cpp'],
>      ['message',                         'message.cpp'],
>      ['object',                          'object.cpp'],
>      ['object-invoke',                   'object-invoke.cpp'],
Laurent Pinchart Jan. 4, 2020, 6:18 p.m. UTC | #2
Hi Niklas,

One additional comment.

On Tue, Dec 17, 2019 at 04:10:32AM +0200, Laurent Pinchart wrote:
> On Tue, Dec 17, 2019 at 02:51:06AM +0100, Niklas Söderlund wrote:
> > Add a test which exercises the whole FileDescriptor interface.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  test/file_descriptor.cpp | 165 +++++++++++++++++++++++++++++++++++++++

I would rename this to file-descriptor.cpp to be consistent with the
other tests.

> >  test/meson.build         |   1 +
> >  2 files changed, 166 insertions(+)
> >  create mode 100644 test/file_descriptor.cpp
> > 
> > diff --git a/test/file_descriptor.cpp b/test/file_descriptor.cpp
> > new file mode 100644
> > index 0000000000000000..177d8bcf16591185
> > --- /dev/null
> > +++ b/test/file_descriptor.cpp
> > @@ -0,0 +1,165 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * file_descriptor.cpp - FileDescriptor test
> > + */
> > +
> > +#include <fcntl.h>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +
> > +#include <libcamera/file_descriptor.h>
> > +
> > +#include "test.h"
> > +
> > +using namespace libcamera;
> > +using namespace std;
> > +
> > +class FileDescriptorTest : public Test
> > +{
> > +protected:
> > +	int init()
> > +	{
> > +		desc1_ = nullptr;
> > +		desc2_ = nullptr;
> > +
> > +		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 FileDescriptor from numerical file descriptor. */
> > +		desc1_ = new FileDescriptor(fd_);
> > +		if (desc1_->fd() == fd_)
> > +			return TestFail;
> > +
> > +		if (!isValidFd(fd_) || !isValidFd(desc1_->fd()))
> > +			return TestFail;
> > +
> > +		int fd = desc1_->fd();
> > +
> > +		delete desc1_;
> > +		desc1_ = nullptr;
> > +
> > +		if (!isValidFd(fd_) || isValidFd(fd))
> > +			return TestFail;
> > +
> > +		/* Test creating FileDescriptor from other FileDescriptor. */
> > +		desc1_ = new FileDescriptor(fd_);
> > +		desc2_ = new FileDescriptor(*desc1_);
> > +
> > +		if (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() == desc2_->fd())
> > +			return TestFail;
> > +
> > +		if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))
> > +			return TestFail;
> > +
> > +		delete desc1_;
> > +		desc1_ = nullptr;
> > +
> > +		if (!isValidFd(desc2_->fd()))
> > +			return TestFail;
> > +
> > +		delete desc2_;
> > +		desc2_ = nullptr;
> > +
> > +		/* Test creating FileDescriptor by taking over other FileDescriptor. */
> > +		desc1_ = new FileDescriptor(fd_);
> > +		fd = desc1_->fd();
> > +		desc2_ = new FileDescriptor(std::move(*desc1_));
> > +
> > +		if (desc1_->fd() != -1 || desc2_->fd() != fd)
> > +			return TestFail;
> > +
> > +		if (!isValidFd(desc2_->fd()))
> > +			return TestFail;
> > +
> > +		delete desc1_;
> > +		desc1_ = nullptr;
> > +		delete desc2_;
> > +		desc2_ = nullptr;
> > +
> > +		/* Test creating FileDescriptor by copy assignment. */
> > +		desc1_ = new FileDescriptor();
> > +		desc2_ = new FileDescriptor(fd_);
> > +
> > +		if (desc1_->fd() != -1)
> > +			return TestFail;
> > +
> > +		fd = desc2_->fd();
> > +		*desc1_ = *desc2_;
> > +
> > +		if (desc1_->fd() == fd || desc2_->fd() != fd)
> > +			return TestFail;
> > +
> > +		if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))
> > +			return TestFail;
> > +
> > +		delete desc1_;
> > +		desc1_ = nullptr;
> > +		delete desc2_;
> > +		desc2_ = nullptr;
> > +
> > +		if (!isValidFd(fd_))
> > +			return TestFail;
> 
> You can drop this check.
> 
> > +
> > +		/* Test creating FileDescriptor by move assignment. */
> > +		desc1_ = new FileDescriptor();
> > +		desc2_ = new FileDescriptor(fd_);
> > +
> > +		fd = desc2_->fd();
> > +		*desc1_ = std::move(*desc2_);
> > +
> > +		if (desc1_->fd() != fd || desc2_->fd() != -1)
> > +			return TestFail;
> > +
> > +		if (!isValidFd(desc1_->fd()))
> > +			return TestFail;
> > +
> > +		delete desc1_;
> > +		desc1_ = nullptr;
> > +		delete desc2_;
> > +		desc2_ = nullptr;
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	void cleanup()
> > +	{
> > +		delete desc2_;
> > +		delete desc1_;
> > +
> > +		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_;
> > +	FileDescriptor *desc1_, *desc2_;
> > +};
> > +
> > +TEST_REGISTER(FileDescriptorTest)
> > diff --git a/test/meson.build b/test/meson.build
> > index 1bb2161dc05a7b1d..9dc392df30efd956 100644
> > --- a/test/meson.build
> > +++ b/test/meson.build
> > @@ -25,6 +25,7 @@ internal_tests = [
> >      ['event',                           'event.cpp'],
> >      ['event-dispatcher',                'event-dispatcher.cpp'],
> >      ['event-thread',                    'event-thread.cpp'],
> > +    ['file_descriptor',                 'file_descriptor.cpp'],
> >      ['message',                         'message.cpp'],
> >      ['object',                          'object.cpp'],
> >      ['object-invoke',                   'object-invoke.cpp'],

Patch

diff --git a/test/file_descriptor.cpp b/test/file_descriptor.cpp
new file mode 100644
index 0000000000000000..177d8bcf16591185
--- /dev/null
+++ b/test/file_descriptor.cpp
@@ -0,0 +1,165 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * file_descriptor.cpp - FileDescriptor test
+ */
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <libcamera/file_descriptor.h>
+
+#include "test.h"
+
+using namespace libcamera;
+using namespace std;
+
+class FileDescriptorTest : public Test
+{
+protected:
+	int init()
+	{
+		desc1_ = nullptr;
+		desc2_ = nullptr;
+
+		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 FileDescriptor from numerical file descriptor. */
+		desc1_ = new FileDescriptor(fd_);
+		if (desc1_->fd() == fd_)
+			return TestFail;
+
+		if (!isValidFd(fd_) || !isValidFd(desc1_->fd()))
+			return TestFail;
+
+		int fd = desc1_->fd();
+
+		delete desc1_;
+		desc1_ = nullptr;
+
+		if (!isValidFd(fd_) || isValidFd(fd))
+			return TestFail;
+
+		/* Test creating FileDescriptor from other FileDescriptor. */
+		desc1_ = new FileDescriptor(fd_);
+		desc2_ = new FileDescriptor(*desc1_);
+
+		if (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() == desc2_->fd())
+			return TestFail;
+
+		if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))
+			return TestFail;
+
+		delete desc1_;
+		desc1_ = nullptr;
+
+		if (!isValidFd(desc2_->fd()))
+			return TestFail;
+
+		delete desc2_;
+		desc2_ = nullptr;
+
+		/* Test creating FileDescriptor by taking over other FileDescriptor. */
+		desc1_ = new FileDescriptor(fd_);
+		fd = desc1_->fd();
+		desc2_ = new FileDescriptor(std::move(*desc1_));
+
+		if (desc1_->fd() != -1 || desc2_->fd() != fd)
+			return TestFail;
+
+		if (!isValidFd(desc2_->fd()))
+			return TestFail;
+
+		delete desc1_;
+		desc1_ = nullptr;
+		delete desc2_;
+		desc2_ = nullptr;
+
+		/* Test creating FileDescriptor by copy assignment. */
+		desc1_ = new FileDescriptor();
+		desc2_ = new FileDescriptor(fd_);
+
+		if (desc1_->fd() != -1)
+			return TestFail;
+
+		fd = desc2_->fd();
+		*desc1_ = *desc2_;
+
+		if (desc1_->fd() == fd || desc2_->fd() != fd)
+			return TestFail;
+
+		if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd()))
+			return TestFail;
+
+		delete desc1_;
+		desc1_ = nullptr;
+		delete desc2_;
+		desc2_ = nullptr;
+
+		if (!isValidFd(fd_))
+			return TestFail;
+
+		/* Test creating FileDescriptor by move assignment. */
+		desc1_ = new FileDescriptor();
+		desc2_ = new FileDescriptor(fd_);
+
+		fd = desc2_->fd();
+		*desc1_ = std::move(*desc2_);
+
+		if (desc1_->fd() != fd || desc2_->fd() != -1)
+			return TestFail;
+
+		if (!isValidFd(desc1_->fd()))
+			return TestFail;
+
+		delete desc1_;
+		desc1_ = nullptr;
+		delete desc2_;
+		desc2_ = nullptr;
+
+		return TestPass;
+	}
+
+	void cleanup()
+	{
+		delete desc2_;
+		delete desc1_;
+
+		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_;
+	FileDescriptor *desc1_, *desc2_;
+};
+
+TEST_REGISTER(FileDescriptorTest)
diff --git a/test/meson.build b/test/meson.build
index 1bb2161dc05a7b1d..9dc392df30efd956 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -25,6 +25,7 @@  internal_tests = [
     ['event',                           'event.cpp'],
     ['event-dispatcher',                'event-dispatcher.cpp'],
     ['event-thread',                    'event-thread.cpp'],
+    ['file_descriptor',                 'file_descriptor.cpp'],
     ['message',                         'message.cpp'],
     ['object',                          'object.cpp'],
     ['object-invoke',                   'object-invoke.cpp'],