| Message ID | 20260128145649.890107-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Wed, Jan 28, 2026 at 03:56:49PM +0100, Barnabás Pőcze wrote: > `O_TMPFILE` requires file system support, which may not be available in > certain environments, usually containerized ones. So do not use it. > > In the ipc/unixsocket and log/log_api tests the temporary file is replaced > with a memfd using the `libcamera::MemFd` class since these tests are > already internal tests. > > In the unique-fd and shared-fd tests instead of creating a temporary file > simply open "/proc/self/exe", which should be available and readable in > most cases. That really feels like a hack (maybe because it's really a hack). Can't we have a TemporaryFile class in libtest instead ? If we centralize the implementation, using memfd without the MemFd class won't be difficult. > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > test/ipc/unixsocket.cpp | 34 ++++++++++++++-------------------- > test/log/log_api.cpp | 14 ++++++-------- > test/shared-fd.cpp | 2 +- > test/unique-fd.cpp | 2 +- > 4 files changed, 22 insertions(+), 30 deletions(-) > > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp > index f39bd986b..28864b829 100644 > --- a/test/ipc/unixsocket.cpp > +++ b/test/ipc/unixsocket.cpp > @@ -18,6 +18,7 @@ > #include <vector> > > #include <libcamera/base/event_dispatcher.h> > +#include <libcamera/base/memfd.h> > #include <libcamera/base/thread.h> > #include <libcamera/base/timer.h> > > @@ -137,11 +138,11 @@ private: > } > > case CMD_JOIN: { > - int outfd = open("/tmp", O_TMPFILE | O_RDWR, > - S_IRUSR | S_IWUSR); > - if (outfd < 0) { > + auto outfd = MemFd::create("test", 0); > + if (!outfd.isValid()) { > + ret = errno; > cerr << "Create out file failed" << endl; > - stop(outfd); > + stop(ret); > return; > } > > @@ -152,15 +153,13 @@ private: > > if (num < 0) { > cerr << "Read failed" << endl; > - close(outfd); > stop(-EIO); > return; > } else if (!num) > break; > > - if (write(outfd, buf, num) < 0) { > + if (write(outfd.get(), buf, num) < 0) { > cerr << "Write failed" << endl; > - close(outfd); > stop(-EIO); > return; > } > @@ -169,9 +168,9 @@ private: > close(fd); > } > > - lseek(outfd, 0, 0); > + lseek(outfd.get(), 0, SEEK_SET); > response.data.push_back(CMD_JOIN); > - response.fds.push_back(outfd); > + response.fds.push_back(outfd.get()); > > ret = ipc_.send(response); > if (ret < 0) { > @@ -179,8 +178,6 @@ private: > stop(ret); > } > > - close(outfd); > - > break; > } > > @@ -315,22 +312,21 @@ protected: > "Foo", > "Bar", > }; > - int fds[2]; > + std::array<UniqueFD, 2> fds; > > for (unsigned int i = 0; i < std::size(strings); i++) { > unsigned int len = strlen(strings[i]); > > - fds[i] = open("/tmp", O_TMPFILE | O_RDWR, > - S_IRUSR | S_IWUSR); > - if (fds[i] < 0) > + fds[i] = MemFd::create("test", 0); > + if (!fds[i].isValid()) > return TestFail; > > - ret = write(fds[i], strings[i], len); > + ret = write(fds[i].get(), strings[i], len); > if (ret < 0) > return TestFail; > > - lseek(fds[i], 0, 0); > - message.fds.push_back(fds[i]); > + lseek(fds[i].get(), 0, SEEK_SET); > + message.fds.push_back(fds[i].get()); > } > > message.data.push_back(CMD_JOIN); > @@ -343,8 +339,6 @@ protected: > unsigned int len = strlen(strings[i]); > std::vector<char> buf(len); > > - close(fds[i]); > - > if (read(response.fds[0], buf.data(), len) <= 0) > return TestFail; > > diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp > index 8d19cf0ce..0eaf67a71 100644 > --- a/test/log/log_api.cpp > +++ b/test/log/log_api.cpp > @@ -17,6 +17,7 @@ > #include <unistd.h> > > #include <libcamera/base/log.h> > +#include <libcamera/base/memfd.h> > > #include <libcamera/logging.h> > > @@ -109,18 +110,17 @@ protected: > > int testFile() > { > - int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > - if (fd < 0) { > + auto fd = MemFd::create("test", 0); > + if (!fd.isValid()) { > cerr << "Failed to open tmp log file" << endl; > return TestFail; > } > > char path[32]; > - snprintf(path, sizeof(path), "/proc/self/fd/%u", fd); > + snprintf(path, sizeof(path), "/proc/self/fd/%u", fd.get()); > > if (logSetFile(path) < 0) { > cerr << "Failed to set log file" << endl; > - close(fd); > return TestFail; > } > > @@ -128,13 +128,11 @@ protected: > > char buf[1000]; > memset(buf, 0, sizeof(buf)); > - lseek(fd, 0, SEEK_SET); > - if (read(fd, buf, sizeof(buf)) < 0) { > + lseek(fd.get(), 0, SEEK_SET); > + if (read(fd.get(), buf, sizeof(buf)) < 0) { > cerr << "Failed to read tmp log file" << endl; > - close(fd); > return TestFail; > } > - close(fd); > > istringstream iss(buf); > return verifyOutput(iss); > diff --git a/test/shared-fd.cpp b/test/shared-fd.cpp > index 57199dfe7..09634f2c2 100644 > --- a/test/shared-fd.cpp > +++ b/test/shared-fd.cpp > @@ -27,7 +27,7 @@ protected: > desc1_ = nullptr; > desc2_ = nullptr; > > - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > + fd_ = open("/proc/self/exe", O_RDONLY); > if (fd_ < 0) > return TestFail; > > diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp > index e556439ea..7d7ba1447 100644 > --- a/test/unique-fd.cpp > +++ b/test/unique-fd.cpp > @@ -189,7 +189,7 @@ protected: > private: > int createFd() > { > - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > + fd_ = open("/proc/self/exe", O_RDONLY); > if (fd_ < 0) > return TestFail; >
2026. 01. 28. 16:04 keltezéssel, Laurent Pinchart írta: > On Wed, Jan 28, 2026 at 03:56:49PM +0100, Barnabás Pőcze wrote: >> `O_TMPFILE` requires file system support, which may not be available in >> certain environments, usually containerized ones. So do not use it. >> >> In the ipc/unixsocket and log/log_api tests the temporary file is replaced >> with a memfd using the `libcamera::MemFd` class since these tests are >> already internal tests. >> >> In the unique-fd and shared-fd tests instead of creating a temporary file >> simply open "/proc/self/exe", which should be available and readable in >> most cases. > > That really feels like a hack (maybe because it's really a hack). Can't > we have a TemporaryFile class in libtest instead ? If we centralize the > implementation, using memfd without the MemFd class won't be difficult. I was also considering using one end of a pipe, one of the standard streams, or simply opening "/tmp" / "." / etc with O_DIRECTORY. A `TemporaryFile` class is certainly an option, but: * arguably none of the tests here need "real temporary files" * in the ipc/unixsocket and log/log_api tests simple pipes could work as far as I can tell * in the unique-fd and shared-fd tests no file is actually needed, only a file descriptor that can be `stat()`-ed * it seems suboptimal to reimplement what's already in the MemFd class That's why I did not want to do that as a first attempt. > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> test/ipc/unixsocket.cpp | 34 ++++++++++++++-------------------- >> test/log/log_api.cpp | 14 ++++++-------- >> test/shared-fd.cpp | 2 +- >> test/unique-fd.cpp | 2 +- >> 4 files changed, 22 insertions(+), 30 deletions(-) >> >> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp >> index f39bd986b..28864b829 100644 >> --- a/test/ipc/unixsocket.cpp >> +++ b/test/ipc/unixsocket.cpp >> @@ -18,6 +18,7 @@ >> #include <vector> >> >> #include <libcamera/base/event_dispatcher.h> >> +#include <libcamera/base/memfd.h> >> #include <libcamera/base/thread.h> >> #include <libcamera/base/timer.h> >> >> @@ -137,11 +138,11 @@ private: >> } >> >> case CMD_JOIN: { >> - int outfd = open("/tmp", O_TMPFILE | O_RDWR, >> - S_IRUSR | S_IWUSR); >> - if (outfd < 0) { >> + auto outfd = MemFd::create("test", 0); >> + if (!outfd.isValid()) { >> + ret = errno; >> cerr << "Create out file failed" << endl; >> - stop(outfd); >> + stop(ret); >> return; >> } >> >> @@ -152,15 +153,13 @@ private: >> >> if (num < 0) { >> cerr << "Read failed" << endl; >> - close(outfd); >> stop(-EIO); >> return; >> } else if (!num) >> break; >> >> - if (write(outfd, buf, num) < 0) { >> + if (write(outfd.get(), buf, num) < 0) { >> cerr << "Write failed" << endl; >> - close(outfd); >> stop(-EIO); >> return; >> } >> @@ -169,9 +168,9 @@ private: >> close(fd); >> } >> >> - lseek(outfd, 0, 0); >> + lseek(outfd.get(), 0, SEEK_SET); >> response.data.push_back(CMD_JOIN); >> - response.fds.push_back(outfd); >> + response.fds.push_back(outfd.get()); >> >> ret = ipc_.send(response); >> if (ret < 0) { >> @@ -179,8 +178,6 @@ private: >> stop(ret); >> } >> >> - close(outfd); >> - >> break; >> } >> >> @@ -315,22 +312,21 @@ protected: >> "Foo", >> "Bar", >> }; >> - int fds[2]; >> + std::array<UniqueFD, 2> fds; >> >> for (unsigned int i = 0; i < std::size(strings); i++) { >> unsigned int len = strlen(strings[i]); >> >> - fds[i] = open("/tmp", O_TMPFILE | O_RDWR, >> - S_IRUSR | S_IWUSR); >> - if (fds[i] < 0) >> + fds[i] = MemFd::create("test", 0); >> + if (!fds[i].isValid()) >> return TestFail; >> >> - ret = write(fds[i], strings[i], len); >> + ret = write(fds[i].get(), strings[i], len); >> if (ret < 0) >> return TestFail; >> >> - lseek(fds[i], 0, 0); >> - message.fds.push_back(fds[i]); >> + lseek(fds[i].get(), 0, SEEK_SET); >> + message.fds.push_back(fds[i].get()); >> } >> >> message.data.push_back(CMD_JOIN); >> @@ -343,8 +339,6 @@ protected: >> unsigned int len = strlen(strings[i]); >> std::vector<char> buf(len); >> >> - close(fds[i]); >> - >> if (read(response.fds[0], buf.data(), len) <= 0) >> return TestFail; >> >> diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp >> index 8d19cf0ce..0eaf67a71 100644 >> --- a/test/log/log_api.cpp >> +++ b/test/log/log_api.cpp >> @@ -17,6 +17,7 @@ >> #include <unistd.h> >> >> #include <libcamera/base/log.h> >> +#include <libcamera/base/memfd.h> >> >> #include <libcamera/logging.h> >> >> @@ -109,18 +110,17 @@ protected: >> >> int testFile() >> { >> - int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); >> - if (fd < 0) { >> + auto fd = MemFd::create("test", 0); >> + if (!fd.isValid()) { >> cerr << "Failed to open tmp log file" << endl; >> return TestFail; >> } >> >> char path[32]; >> - snprintf(path, sizeof(path), "/proc/self/fd/%u", fd); >> + snprintf(path, sizeof(path), "/proc/self/fd/%u", fd.get()); >> >> if (logSetFile(path) < 0) { >> cerr << "Failed to set log file" << endl; >> - close(fd); >> return TestFail; >> } >> >> @@ -128,13 +128,11 @@ protected: >> >> char buf[1000]; >> memset(buf, 0, sizeof(buf)); >> - lseek(fd, 0, SEEK_SET); >> - if (read(fd, buf, sizeof(buf)) < 0) { >> + lseek(fd.get(), 0, SEEK_SET); >> + if (read(fd.get(), buf, sizeof(buf)) < 0) { >> cerr << "Failed to read tmp log file" << endl; >> - close(fd); >> return TestFail; >> } >> - close(fd); >> >> istringstream iss(buf); >> return verifyOutput(iss); >> diff --git a/test/shared-fd.cpp b/test/shared-fd.cpp >> index 57199dfe7..09634f2c2 100644 >> --- a/test/shared-fd.cpp >> +++ b/test/shared-fd.cpp >> @@ -27,7 +27,7 @@ protected: >> desc1_ = nullptr; >> desc2_ = nullptr; >> >> - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); >> + fd_ = open("/proc/self/exe", O_RDONLY); >> if (fd_ < 0) >> return TestFail; >> >> diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp >> index e556439ea..7d7ba1447 100644 >> --- a/test/unique-fd.cpp >> +++ b/test/unique-fd.cpp >> @@ -189,7 +189,7 @@ protected: >> private: >> int createFd() >> { >> - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); >> + fd_ = open("/proc/self/exe", O_RDONLY); >> if (fd_ < 0) >> return TestFail; >> >
2026. 01. 28. 16:13 keltezéssel, Barnabás Pőcze írta: > 2026. 01. 28. 16:04 keltezéssel, Laurent Pinchart írta: >> On Wed, Jan 28, 2026 at 03:56:49PM +0100, Barnabás Pőcze wrote: >>> `O_TMPFILE` requires file system support, which may not be available in >>> certain environments, usually containerized ones. So do not use it. >>> >>> In the ipc/unixsocket and log/log_api tests the temporary file is replaced >>> with a memfd using the `libcamera::MemFd` class since these tests are >>> already internal tests. >>> >>> In the unique-fd and shared-fd tests instead of creating a temporary file >>> simply open "/proc/self/exe", which should be available and readable in >>> most cases. >> >> That really feels like a hack (maybe because it's really a hack). Can't >> we have a TemporaryFile class in libtest instead ? If we centralize the >> implementation, using memfd without the MemFd class won't be difficult. > > I was also considering using one end of a pipe, one of the standard streams, > or simply opening "/tmp" / "." / etc with O_DIRECTORY. > > A `TemporaryFile` class is certainly an option, but: > * arguably none of the tests here need "real temporary files" > * in the ipc/unixsocket and log/log_api tests > simple pipes could work as far as I can tell > * in the unique-fd and shared-fd tests no file is actually needed, > only a file descriptor that can be `stat()`-ed > * it seems suboptimal to reimplement what's already in the MemFd class > > That's why I did not want to do that as a first attempt. > I see that "libtest" can use the internal parts. So I suppose I can add a `createTemporaryFile()` function that just returns `MemFd::create("libcamera-test-temp-file", 0)` if opening "/proc/self/exe" and similar are considered too problematic workarounds. (It's not clear to me what interface a `TemporaryFile` class would have (unless it's just a single static function to create one).) > >> >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>> --- >>> test/ipc/unixsocket.cpp | 34 ++++++++++++++-------------------- >>> test/log/log_api.cpp | 14 ++++++-------- >>> test/shared-fd.cpp | 2 +- >>> test/unique-fd.cpp | 2 +- >>> 4 files changed, 22 insertions(+), 30 deletions(-) >>> >>> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp >>> index f39bd986b..28864b829 100644 >>> --- a/test/ipc/unixsocket.cpp >>> +++ b/test/ipc/unixsocket.cpp >>> @@ -18,6 +18,7 @@ >>> #include <vector> >>> #include <libcamera/base/event_dispatcher.h> >>> +#include <libcamera/base/memfd.h> >>> #include <libcamera/base/thread.h> >>> #include <libcamera/base/timer.h> >>> @@ -137,11 +138,11 @@ private: >>> } >>> case CMD_JOIN: { >>> - int outfd = open("/tmp", O_TMPFILE | O_RDWR, >>> - S_IRUSR | S_IWUSR); >>> - if (outfd < 0) { >>> + auto outfd = MemFd::create("test", 0); >>> + if (!outfd.isValid()) { >>> + ret = errno; >>> cerr << "Create out file failed" << endl; >>> - stop(outfd); >>> + stop(ret); >>> return; >>> } >>> @@ -152,15 +153,13 @@ private: >>> if (num < 0) { >>> cerr << "Read failed" << endl; >>> - close(outfd); >>> stop(-EIO); >>> return; >>> } else if (!num) >>> break; >>> - if (write(outfd, buf, num) < 0) { >>> + if (write(outfd.get(), buf, num) < 0) { >>> cerr << "Write failed" << endl; >>> - close(outfd); >>> stop(-EIO); >>> return; >>> } >>> @@ -169,9 +168,9 @@ private: >>> close(fd); >>> } >>> - lseek(outfd, 0, 0); >>> + lseek(outfd.get(), 0, SEEK_SET); >>> response.data.push_back(CMD_JOIN); >>> - response.fds.push_back(outfd); >>> + response.fds.push_back(outfd.get()); >>> ret = ipc_.send(response); >>> if (ret < 0) { >>> @@ -179,8 +178,6 @@ private: >>> stop(ret); >>> } >>> - close(outfd); >>> - >>> break; >>> } >>> @@ -315,22 +312,21 @@ protected: >>> "Foo", >>> "Bar", >>> }; >>> - int fds[2]; >>> + std::array<UniqueFD, 2> fds; >>> for (unsigned int i = 0; i < std::size(strings); i++) { >>> unsigned int len = strlen(strings[i]); >>> - fds[i] = open("/tmp", O_TMPFILE | O_RDWR, >>> - S_IRUSR | S_IWUSR); >>> - if (fds[i] < 0) >>> + fds[i] = MemFd::create("test", 0); >>> + if (!fds[i].isValid()) >>> return TestFail; >>> - ret = write(fds[i], strings[i], len); >>> + ret = write(fds[i].get(), strings[i], len); >>> if (ret < 0) >>> return TestFail; >>> - lseek(fds[i], 0, 0); >>> - message.fds.push_back(fds[i]); >>> + lseek(fds[i].get(), 0, SEEK_SET); >>> + message.fds.push_back(fds[i].get()); >>> } >>> message.data.push_back(CMD_JOIN); >>> @@ -343,8 +339,6 @@ protected: >>> unsigned int len = strlen(strings[i]); >>> std::vector<char> buf(len); >>> - close(fds[i]); >>> - >>> if (read(response.fds[0], buf.data(), len) <= 0) >>> return TestFail; >>> diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp >>> index 8d19cf0ce..0eaf67a71 100644 >>> --- a/test/log/log_api.cpp >>> +++ b/test/log/log_api.cpp >>> @@ -17,6 +17,7 @@ >>> #include <unistd.h> >>> #include <libcamera/base/log.h> >>> +#include <libcamera/base/memfd.h> >>> #include <libcamera/logging.h> >>> @@ -109,18 +110,17 @@ protected: >>> int testFile() >>> { >>> - int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); >>> - if (fd < 0) { >>> + auto fd = MemFd::create("test", 0); >>> + if (!fd.isValid()) { >>> cerr << "Failed to open tmp log file" << endl; >>> return TestFail; >>> } >>> char path[32]; >>> - snprintf(path, sizeof(path), "/proc/self/fd/%u", fd); >>> + snprintf(path, sizeof(path), "/proc/self/fd/%u", fd.get()); >>> if (logSetFile(path) < 0) { >>> cerr << "Failed to set log file" << endl; >>> - close(fd); >>> return TestFail; >>> } >>> @@ -128,13 +128,11 @@ protected: >>> char buf[1000]; >>> memset(buf, 0, sizeof(buf)); >>> - lseek(fd, 0, SEEK_SET); >>> - if (read(fd, buf, sizeof(buf)) < 0) { >>> + lseek(fd.get(), 0, SEEK_SET); >>> + if (read(fd.get(), buf, sizeof(buf)) < 0) { >>> cerr << "Failed to read tmp log file" << endl; >>> - close(fd); >>> return TestFail; >>> } >>> - close(fd); >>> istringstream iss(buf); >>> return verifyOutput(iss); >>> diff --git a/test/shared-fd.cpp b/test/shared-fd.cpp >>> index 57199dfe7..09634f2c2 100644 >>> --- a/test/shared-fd.cpp >>> +++ b/test/shared-fd.cpp >>> @@ -27,7 +27,7 @@ protected: >>> desc1_ = nullptr; >>> desc2_ = nullptr; >>> - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); >>> + fd_ = open("/proc/self/exe", O_RDONLY); >>> if (fd_ < 0) >>> return TestFail; >>> diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp >>> index e556439ea..7d7ba1447 100644 >>> --- a/test/unique-fd.cpp >>> +++ b/test/unique-fd.cpp >>> @@ -189,7 +189,7 @@ protected: >>> private: >>> int createFd() >>> { >>> - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); >>> + fd_ = open("/proc/self/exe", O_RDONLY); >>> if (fd_ < 0) >>> return TestFail; >> >
On Wed, Jan 28, 2026 at 04:13:34PM +0100, Barnabás Pőcze wrote: > 2026. 01. 28. 16:04 keltezéssel, Laurent Pinchart írta: > > On Wed, Jan 28, 2026 at 03:56:49PM +0100, Barnabás Pőcze wrote: > >> `O_TMPFILE` requires file system support, which may not be available in > >> certain environments, usually containerized ones. So do not use it. > >> > >> In the ipc/unixsocket and log/log_api tests the temporary file is replaced > >> with a memfd using the `libcamera::MemFd` class since these tests are > >> already internal tests. > >> > >> In the unique-fd and shared-fd tests instead of creating a temporary file > >> simply open "/proc/self/exe", which should be available and readable in > >> most cases. > > > > That really feels like a hack (maybe because it's really a hack). Can't > > we have a TemporaryFile class in libtest instead ? If we centralize the > > implementation, using memfd without the MemFd class won't be difficult. > > I was also considering using one end of a pipe, one of the standard streams, > or simply opening "/tmp" / "." / etc with O_DIRECTORY. > > A `TemporaryFile` class is certainly an option, but: > * arguably none of the tests here need "real temporary files" > * in the ipc/unixsocket and log/log_api tests > simple pipes could work as far as I can tell > * in the unique-fd and shared-fd tests no file is actually needed, > only a file descriptor that can be `stat()`-ed > * it seems suboptimal to reimplement what's already in the MemFd class > > That's why I did not want to do that as a first attempt. What I like about a TemporaryFile class in libtest is that it would centralize the implementation and abstract the underlying API. I think it would make the tests simpler, there would be less to think about in the unit tests code. > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> --- > >> test/ipc/unixsocket.cpp | 34 ++++++++++++++-------------------- > >> test/log/log_api.cpp | 14 ++++++-------- > >> test/shared-fd.cpp | 2 +- > >> test/unique-fd.cpp | 2 +- > >> 4 files changed, 22 insertions(+), 30 deletions(-) > >> > >> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp > >> index f39bd986b..28864b829 100644 > >> --- a/test/ipc/unixsocket.cpp > >> +++ b/test/ipc/unixsocket.cpp > >> @@ -18,6 +18,7 @@ > >> #include <vector> > >> > >> #include <libcamera/base/event_dispatcher.h> > >> +#include <libcamera/base/memfd.h> > >> #include <libcamera/base/thread.h> > >> #include <libcamera/base/timer.h> > >> > >> @@ -137,11 +138,11 @@ private: > >> } > >> > >> case CMD_JOIN: { > >> - int outfd = open("/tmp", O_TMPFILE | O_RDWR, > >> - S_IRUSR | S_IWUSR); > >> - if (outfd < 0) { > >> + auto outfd = MemFd::create("test", 0); > >> + if (!outfd.isValid()) { > >> + ret = errno; > >> cerr << "Create out file failed" << endl; > >> - stop(outfd); > >> + stop(ret); > >> return; > >> } > >> > >> @@ -152,15 +153,13 @@ private: > >> > >> if (num < 0) { > >> cerr << "Read failed" << endl; > >> - close(outfd); > >> stop(-EIO); > >> return; > >> } else if (!num) > >> break; > >> > >> - if (write(outfd, buf, num) < 0) { > >> + if (write(outfd.get(), buf, num) < 0) { > >> cerr << "Write failed" << endl; > >> - close(outfd); > >> stop(-EIO); > >> return; > >> } > >> @@ -169,9 +168,9 @@ private: > >> close(fd); > >> } > >> > >> - lseek(outfd, 0, 0); > >> + lseek(outfd.get(), 0, SEEK_SET); > >> response.data.push_back(CMD_JOIN); > >> - response.fds.push_back(outfd); > >> + response.fds.push_back(outfd.get()); > >> > >> ret = ipc_.send(response); > >> if (ret < 0) { > >> @@ -179,8 +178,6 @@ private: > >> stop(ret); > >> } > >> > >> - close(outfd); > >> - > >> break; > >> } > >> > >> @@ -315,22 +312,21 @@ protected: > >> "Foo", > >> "Bar", > >> }; > >> - int fds[2]; > >> + std::array<UniqueFD, 2> fds; > >> > >> for (unsigned int i = 0; i < std::size(strings); i++) { > >> unsigned int len = strlen(strings[i]); > >> > >> - fds[i] = open("/tmp", O_TMPFILE | O_RDWR, > >> - S_IRUSR | S_IWUSR); > >> - if (fds[i] < 0) > >> + fds[i] = MemFd::create("test", 0); > >> + if (!fds[i].isValid()) > >> return TestFail; > >> > >> - ret = write(fds[i], strings[i], len); > >> + ret = write(fds[i].get(), strings[i], len); > >> if (ret < 0) > >> return TestFail; > >> > >> - lseek(fds[i], 0, 0); > >> - message.fds.push_back(fds[i]); > >> + lseek(fds[i].get(), 0, SEEK_SET); > >> + message.fds.push_back(fds[i].get()); > >> } > >> > >> message.data.push_back(CMD_JOIN); > >> @@ -343,8 +339,6 @@ protected: > >> unsigned int len = strlen(strings[i]); > >> std::vector<char> buf(len); > >> > >> - close(fds[i]); > >> - > >> if (read(response.fds[0], buf.data(), len) <= 0) > >> return TestFail; > >> > >> diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp > >> index 8d19cf0ce..0eaf67a71 100644 > >> --- a/test/log/log_api.cpp > >> +++ b/test/log/log_api.cpp > >> @@ -17,6 +17,7 @@ > >> #include <unistd.h> > >> > >> #include <libcamera/base/log.h> > >> +#include <libcamera/base/memfd.h> > >> > >> #include <libcamera/logging.h> > >> > >> @@ -109,18 +110,17 @@ protected: > >> > >> int testFile() > >> { > >> - int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > >> - if (fd < 0) { > >> + auto fd = MemFd::create("test", 0); > >> + if (!fd.isValid()) { > >> cerr << "Failed to open tmp log file" << endl; > >> return TestFail; > >> } > >> > >> char path[32]; > >> - snprintf(path, sizeof(path), "/proc/self/fd/%u", fd); > >> + snprintf(path, sizeof(path), "/proc/self/fd/%u", fd.get()); > >> > >> if (logSetFile(path) < 0) { > >> cerr << "Failed to set log file" << endl; > >> - close(fd); > >> return TestFail; > >> } > >> > >> @@ -128,13 +128,11 @@ protected: > >> > >> char buf[1000]; > >> memset(buf, 0, sizeof(buf)); > >> - lseek(fd, 0, SEEK_SET); > >> - if (read(fd, buf, sizeof(buf)) < 0) { > >> + lseek(fd.get(), 0, SEEK_SET); > >> + if (read(fd.get(), buf, sizeof(buf)) < 0) { > >> cerr << "Failed to read tmp log file" << endl; > >> - close(fd); > >> return TestFail; > >> } > >> - close(fd); > >> > >> istringstream iss(buf); > >> return verifyOutput(iss); > >> diff --git a/test/shared-fd.cpp b/test/shared-fd.cpp > >> index 57199dfe7..09634f2c2 100644 > >> --- a/test/shared-fd.cpp > >> +++ b/test/shared-fd.cpp > >> @@ -27,7 +27,7 @@ protected: > >> desc1_ = nullptr; > >> desc2_ = nullptr; > >> > >> - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > >> + fd_ = open("/proc/self/exe", O_RDONLY); > >> if (fd_ < 0) > >> return TestFail; > >> > >> diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp > >> index e556439ea..7d7ba1447 100644 > >> --- a/test/unique-fd.cpp > >> +++ b/test/unique-fd.cpp > >> @@ -189,7 +189,7 @@ protected: > >> private: > >> int createFd() > >> { > >> - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > >> + fd_ = open("/proc/self/exe", O_RDONLY); > >> if (fd_ < 0) > >> return TestFail; > >>
On Wed, Jan 28, 2026 at 05:35:01PM +0100, Barnabás Pőcze wrote: > 2026. 01. 28. 16:13 keltezéssel, Barnabás Pőcze írta: > > 2026. 01. 28. 16:04 keltezéssel, Laurent Pinchart írta: > >> On Wed, Jan 28, 2026 at 03:56:49PM +0100, Barnabás Pőcze wrote: > >>> `O_TMPFILE` requires file system support, which may not be available in > >>> certain environments, usually containerized ones. So do not use it. > >>> > >>> In the ipc/unixsocket and log/log_api tests the temporary file is replaced > >>> with a memfd using the `libcamera::MemFd` class since these tests are > >>> already internal tests. > >>> > >>> In the unique-fd and shared-fd tests instead of creating a temporary file > >>> simply open "/proc/self/exe", which should be available and readable in > >>> most cases. > >> > >> That really feels like a hack (maybe because it's really a hack). Can't > >> we have a TemporaryFile class in libtest instead ? If we centralize the > >> implementation, using memfd without the MemFd class won't be difficult. > > > > I was also considering using one end of a pipe, one of the standard streams, > > or simply opening "/tmp" / "." / etc with O_DIRECTORY. > > > > A `TemporaryFile` class is certainly an option, but: > > * arguably none of the tests here need "real temporary files" > > * in the ipc/unixsocket and log/log_api tests > > simple pipes could work as far as I can tell > > * in the unique-fd and shared-fd tests no file is actually needed, > > only a file descriptor that can be `stat()`-ed > > * it seems suboptimal to reimplement what's already in the MemFd class > > > > That's why I did not want to do that as a first attempt. > > > > I see that "libtest" can use the internal parts. So I suppose I can add > a `createTemporaryFile()` function that just returns `MemFd::create("libcamera-test-temp-file", 0)` > if opening "/proc/self/exe" and similar are considered too problematic workarounds. > (It's not clear to me what interface a `TemporaryFile` class would have > (unless it's just a single static function to create one).) Yes, a single function sounds good. I'm fine using MemFd there as it will be easy to change that if needed. > >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >>> --- > >>> test/ipc/unixsocket.cpp | 34 ++++++++++++++-------------------- > >>> test/log/log_api.cpp | 14 ++++++-------- > >>> test/shared-fd.cpp | 2 +- > >>> test/unique-fd.cpp | 2 +- > >>> 4 files changed, 22 insertions(+), 30 deletions(-) > >>> > >>> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp > >>> index f39bd986b..28864b829 100644 > >>> --- a/test/ipc/unixsocket.cpp > >>> +++ b/test/ipc/unixsocket.cpp > >>> @@ -18,6 +18,7 @@ > >>> #include <vector> > >>> #include <libcamera/base/event_dispatcher.h> > >>> +#include <libcamera/base/memfd.h> > >>> #include <libcamera/base/thread.h> > >>> #include <libcamera/base/timer.h> > >>> @@ -137,11 +138,11 @@ private: > >>> } > >>> case CMD_JOIN: { > >>> - int outfd = open("/tmp", O_TMPFILE | O_RDWR, > >>> - S_IRUSR | S_IWUSR); > >>> - if (outfd < 0) { > >>> + auto outfd = MemFd::create("test", 0); > >>> + if (!outfd.isValid()) { > >>> + ret = errno; > >>> cerr << "Create out file failed" << endl; > >>> - stop(outfd); > >>> + stop(ret); > >>> return; > >>> } > >>> @@ -152,15 +153,13 @@ private: > >>> if (num < 0) { > >>> cerr << "Read failed" << endl; > >>> - close(outfd); > >>> stop(-EIO); > >>> return; > >>> } else if (!num) > >>> break; > >>> - if (write(outfd, buf, num) < 0) { > >>> + if (write(outfd.get(), buf, num) < 0) { > >>> cerr << "Write failed" << endl; > >>> - close(outfd); > >>> stop(-EIO); > >>> return; > >>> } > >>> @@ -169,9 +168,9 @@ private: > >>> close(fd); > >>> } > >>> - lseek(outfd, 0, 0); > >>> + lseek(outfd.get(), 0, SEEK_SET); > >>> response.data.push_back(CMD_JOIN); > >>> - response.fds.push_back(outfd); > >>> + response.fds.push_back(outfd.get()); > >>> ret = ipc_.send(response); > >>> if (ret < 0) { > >>> @@ -179,8 +178,6 @@ private: > >>> stop(ret); > >>> } > >>> - close(outfd); > >>> - > >>> break; > >>> } > >>> @@ -315,22 +312,21 @@ protected: > >>> "Foo", > >>> "Bar", > >>> }; > >>> - int fds[2]; > >>> + std::array<UniqueFD, 2> fds; > >>> for (unsigned int i = 0; i < std::size(strings); i++) { > >>> unsigned int len = strlen(strings[i]); > >>> - fds[i] = open("/tmp", O_TMPFILE | O_RDWR, > >>> - S_IRUSR | S_IWUSR); > >>> - if (fds[i] < 0) > >>> + fds[i] = MemFd::create("test", 0); > >>> + if (!fds[i].isValid()) > >>> return TestFail; > >>> - ret = write(fds[i], strings[i], len); > >>> + ret = write(fds[i].get(), strings[i], len); > >>> if (ret < 0) > >>> return TestFail; > >>> - lseek(fds[i], 0, 0); > >>> - message.fds.push_back(fds[i]); > >>> + lseek(fds[i].get(), 0, SEEK_SET); > >>> + message.fds.push_back(fds[i].get()); > >>> } > >>> message.data.push_back(CMD_JOIN); > >>> @@ -343,8 +339,6 @@ protected: > >>> unsigned int len = strlen(strings[i]); > >>> std::vector<char> buf(len); > >>> - close(fds[i]); > >>> - > >>> if (read(response.fds[0], buf.data(), len) <= 0) > >>> return TestFail; > >>> diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp > >>> index 8d19cf0ce..0eaf67a71 100644 > >>> --- a/test/log/log_api.cpp > >>> +++ b/test/log/log_api.cpp > >>> @@ -17,6 +17,7 @@ > >>> #include <unistd.h> > >>> #include <libcamera/base/log.h> > >>> +#include <libcamera/base/memfd.h> > >>> #include <libcamera/logging.h> > >>> @@ -109,18 +110,17 @@ protected: > >>> int testFile() > >>> { > >>> - int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > >>> - if (fd < 0) { > >>> + auto fd = MemFd::create("test", 0); > >>> + if (!fd.isValid()) { > >>> cerr << "Failed to open tmp log file" << endl; > >>> return TestFail; > >>> } > >>> char path[32]; > >>> - snprintf(path, sizeof(path), "/proc/self/fd/%u", fd); > >>> + snprintf(path, sizeof(path), "/proc/self/fd/%u", fd.get()); > >>> if (logSetFile(path) < 0) { > >>> cerr << "Failed to set log file" << endl; > >>> - close(fd); > >>> return TestFail; > >>> } > >>> @@ -128,13 +128,11 @@ protected: > >>> char buf[1000]; > >>> memset(buf, 0, sizeof(buf)); > >>> - lseek(fd, 0, SEEK_SET); > >>> - if (read(fd, buf, sizeof(buf)) < 0) { > >>> + lseek(fd.get(), 0, SEEK_SET); > >>> + if (read(fd.get(), buf, sizeof(buf)) < 0) { > >>> cerr << "Failed to read tmp log file" << endl; > >>> - close(fd); > >>> return TestFail; > >>> } > >>> - close(fd); > >>> istringstream iss(buf); > >>> return verifyOutput(iss); > >>> diff --git a/test/shared-fd.cpp b/test/shared-fd.cpp > >>> index 57199dfe7..09634f2c2 100644 > >>> --- a/test/shared-fd.cpp > >>> +++ b/test/shared-fd.cpp > >>> @@ -27,7 +27,7 @@ protected: > >>> desc1_ = nullptr; > >>> desc2_ = nullptr; > >>> - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > >>> + fd_ = open("/proc/self/exe", O_RDONLY); > >>> if (fd_ < 0) > >>> return TestFail; > >>> diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp > >>> index e556439ea..7d7ba1447 100644 > >>> --- a/test/unique-fd.cpp > >>> +++ b/test/unique-fd.cpp > >>> @@ -189,7 +189,7 @@ protected: > >>> private: > >>> int createFd() > >>> { > >>> - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); > >>> + fd_ = open("/proc/self/exe", O_RDONLY); > >>> if (fd_ < 0) > >>> return TestFail;
diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp index f39bd986b..28864b829 100644 --- a/test/ipc/unixsocket.cpp +++ b/test/ipc/unixsocket.cpp @@ -18,6 +18,7 @@ #include <vector> #include <libcamera/base/event_dispatcher.h> +#include <libcamera/base/memfd.h> #include <libcamera/base/thread.h> #include <libcamera/base/timer.h> @@ -137,11 +138,11 @@ private: } case CMD_JOIN: { - int outfd = open("/tmp", O_TMPFILE | O_RDWR, - S_IRUSR | S_IWUSR); - if (outfd < 0) { + auto outfd = MemFd::create("test", 0); + if (!outfd.isValid()) { + ret = errno; cerr << "Create out file failed" << endl; - stop(outfd); + stop(ret); return; } @@ -152,15 +153,13 @@ private: if (num < 0) { cerr << "Read failed" << endl; - close(outfd); stop(-EIO); return; } else if (!num) break; - if (write(outfd, buf, num) < 0) { + if (write(outfd.get(), buf, num) < 0) { cerr << "Write failed" << endl; - close(outfd); stop(-EIO); return; } @@ -169,9 +168,9 @@ private: close(fd); } - lseek(outfd, 0, 0); + lseek(outfd.get(), 0, SEEK_SET); response.data.push_back(CMD_JOIN); - response.fds.push_back(outfd); + response.fds.push_back(outfd.get()); ret = ipc_.send(response); if (ret < 0) { @@ -179,8 +178,6 @@ private: stop(ret); } - close(outfd); - break; } @@ -315,22 +312,21 @@ protected: "Foo", "Bar", }; - int fds[2]; + std::array<UniqueFD, 2> fds; for (unsigned int i = 0; i < std::size(strings); i++) { unsigned int len = strlen(strings[i]); - fds[i] = open("/tmp", O_TMPFILE | O_RDWR, - S_IRUSR | S_IWUSR); - if (fds[i] < 0) + fds[i] = MemFd::create("test", 0); + if (!fds[i].isValid()) return TestFail; - ret = write(fds[i], strings[i], len); + ret = write(fds[i].get(), strings[i], len); if (ret < 0) return TestFail; - lseek(fds[i], 0, 0); - message.fds.push_back(fds[i]); + lseek(fds[i].get(), 0, SEEK_SET); + message.fds.push_back(fds[i].get()); } message.data.push_back(CMD_JOIN); @@ -343,8 +339,6 @@ protected: unsigned int len = strlen(strings[i]); std::vector<char> buf(len); - close(fds[i]); - if (read(response.fds[0], buf.data(), len) <= 0) return TestFail; diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp index 8d19cf0ce..0eaf67a71 100644 --- a/test/log/log_api.cpp +++ b/test/log/log_api.cpp @@ -17,6 +17,7 @@ #include <unistd.h> #include <libcamera/base/log.h> +#include <libcamera/base/memfd.h> #include <libcamera/logging.h> @@ -109,18 +110,17 @@ protected: int testFile() { - int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); - if (fd < 0) { + auto fd = MemFd::create("test", 0); + if (!fd.isValid()) { cerr << "Failed to open tmp log file" << endl; return TestFail; } char path[32]; - snprintf(path, sizeof(path), "/proc/self/fd/%u", fd); + snprintf(path, sizeof(path), "/proc/self/fd/%u", fd.get()); if (logSetFile(path) < 0) { cerr << "Failed to set log file" << endl; - close(fd); return TestFail; } @@ -128,13 +128,11 @@ protected: char buf[1000]; memset(buf, 0, sizeof(buf)); - lseek(fd, 0, SEEK_SET); - if (read(fd, buf, sizeof(buf)) < 0) { + lseek(fd.get(), 0, SEEK_SET); + if (read(fd.get(), buf, sizeof(buf)) < 0) { cerr << "Failed to read tmp log file" << endl; - close(fd); return TestFail; } - close(fd); istringstream iss(buf); return verifyOutput(iss); diff --git a/test/shared-fd.cpp b/test/shared-fd.cpp index 57199dfe7..09634f2c2 100644 --- a/test/shared-fd.cpp +++ b/test/shared-fd.cpp @@ -27,7 +27,7 @@ protected: desc1_ = nullptr; desc2_ = nullptr; - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); + fd_ = open("/proc/self/exe", O_RDONLY); if (fd_ < 0) return TestFail; diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp index e556439ea..7d7ba1447 100644 --- a/test/unique-fd.cpp +++ b/test/unique-fd.cpp @@ -189,7 +189,7 @@ protected: private: int createFd() { - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); + fd_ = open("/proc/self/exe", O_RDONLY); if (fd_ < 0) return TestFail;
`O_TMPFILE` requires file system support, which may not be available in certain environments, usually containerized ones. So do not use it. In the ipc/unixsocket and log/log_api tests the temporary file is replaced with a memfd using the `libcamera::MemFd` class since these tests are already internal tests. In the unique-fd and shared-fd tests instead of creating a temporary file simply open "/proc/self/exe", which should be available and readable in most cases. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- test/ipc/unixsocket.cpp | 34 ++++++++++++++-------------------- test/log/log_api.cpp | 14 ++++++-------- test/shared-fd.cpp | 2 +- test/unique-fd.cpp | 2 +- 4 files changed, 22 insertions(+), 30 deletions(-)