@@ -12,6 +12,7 @@
#include <sys/types.h>
#include <vector>
+#include <libcamera/scoped_fd.h>
#include <libcamera/signal.h>
namespace libcamera {
@@ -29,7 +30,7 @@ public:
IPCUnixSocket();
~IPCUnixSocket();
- int create();
+ ScopedFD create();
int bind(int fd);
void close();
bool isBound() const;
@@ -50,7 +51,7 @@ private:
void dataNotifier(EventNotifier *notifier);
- int fd_;
+ ScopedFD fd_;
bool headerReceived_;
struct Header header_;
EventNotifier *notifier_;
@@ -30,14 +30,14 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
args.push_back(ipaModulePath);
socket_ = std::make_unique<IPCUnixSocket>();
- int fd = socket_->create();
- if (fd < 0) {
+ ScopedFD fd = socket_->create();
+ if (!fd.isValid()) {
LOG(IPCPipe, Error) << "Failed to create socket";
return;
}
socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead);
- args.push_back(std::to_string(fd));
- fds.push_back(fd);
+ args.push_back(std::to_string(fd.get()));
+ fds.push_back(fd.release());
proc_ = std::make_unique<Process>();
int ret = proc_->start(ipaProxyWorkerPath, args, fds);
@@ -68,7 +68,7 @@ LOG_DEFINE_CATEGORY(IPCUnixSocket)
*/
IPCUnixSocket::IPCUnixSocket()
- : fd_(-1), headerReceived_(false), notifier_(nullptr)
+ : headerReceived_(false), notifier_(nullptr)
{
}
@@ -86,9 +86,9 @@ IPCUnixSocket::~IPCUnixSocket()
* the remote process, where it can be used with IPCUnixSocket::bind() to bind
* the remote side socket.
*
- * \return A file descriptor on success, negative error code on failure
+ * \return A file descriptor. It is valid on success or invalid otherwise.
*/
-int IPCUnixSocket::create()
+ScopedFD IPCUnixSocket::create()
{
int sockets[2];
int ret;
@@ -98,14 +98,13 @@ int IPCUnixSocket::create()
ret = -errno;
LOG(IPCUnixSocket, Error)
<< "Failed to create socket pair: " << strerror(-ret);
- return ret;
+ return ScopedFD();
}
- ret = bind(sockets[0]);
- if (ret)
- return ret;
+ if (bind(sockets[0]) < 0)
+ return ScopedFD();
- return sockets[1];
+ return ScopedFD(sockets[1]);
}
/**
@@ -116,15 +115,18 @@ int IPCUnixSocket::create()
* by the file descriptor \a fd. The file descriptor is obtained from the
* IPCUnixSocket::create() method.
*
- * \return 0 on success or a negative error code otherwise
+ * \return 0 on success or a negative error code otherwise.
+ *
+ * \todo This argument should be ScopedFD because bind() takes over the
+ * ownership.
*/
int IPCUnixSocket::bind(int fd)
{
if (isBound())
return -EINVAL;
- fd_ = fd;
- notifier_ = new EventNotifier(fd_, EventNotifier::Read);
+ fd_ = ScopedFD(fd);
+ notifier_ = new EventNotifier(fd_.get(), EventNotifier::Read);
notifier_->activated.connect(this, &IPCUnixSocket::dataNotifier);
return 0;
@@ -143,9 +145,7 @@ void IPCUnixSocket::close()
delete notifier_;
notifier_ = nullptr;
- ::close(fd_);
-
- fd_ = -1;
+ fd_.reset();
headerReceived_ = false;
}
@@ -155,7 +155,7 @@ void IPCUnixSocket::close()
*/
bool IPCUnixSocket::isBound() const
{
- return fd_ != -1;
+ return fd_.isValid();
}
/**
@@ -182,7 +182,7 @@ int IPCUnixSocket::send(const Payload &payload)
if (!hdr.data && !hdr.fds)
return -EINVAL;
- ret = ::send(fd_, &hdr, sizeof(hdr), 0);
+ ret = ::send(fd_.get(), &hdr, sizeof(hdr), 0);
if (ret < 0) {
ret = -errno;
LOG(IPCUnixSocket, Error)
@@ -262,7 +262,7 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,
msg.msg_flags = 0;
memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t));
- if (sendmsg(fd_, &msg, 0) < 0) {
+ if (sendmsg(fd_.get(), &msg, 0) < 0) {
int ret = -errno;
LOG(IPCUnixSocket, Error)
<< "Failed to sendmsg: " << strerror(-ret);
@@ -296,7 +296,7 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,
msg.msg_controllen = cmsg->cmsg_len;
msg.msg_flags = 0;
- if (recvmsg(fd_, &msg, 0) < 0) {
+ if (recvmsg(fd_.get(), &msg, 0) < 0) {
int ret = -errno;
if (ret != -EAGAIN)
LOG(IPCUnixSocket, Error)
@@ -315,7 +315,7 @@ void IPCUnixSocket::dataNotifier([[maybe_unused]] EventNotifier *notifier)
if (!headerReceived_) {
/* Receive the header. */
- ret = ::recv(fd_, &header_, sizeof(header_), 0);
+ ret = ::recv(fd_.get(), &header_, sizeof(header_), 0);
if (ret < 0) {
ret = -errno;
LOG(IPCUnixSocket, Error)
@@ -331,7 +331,7 @@ void IPCUnixSocket::dataNotifier([[maybe_unused]] EventNotifier *notifier)
* readyRead signal. The notifier will be reenabled by the receive()
* method.
*/
- struct pollfd fds = { fd_, POLLIN, 0 };
+ struct pollfd fds = { fd_.get(), POLLIN, 0 };
ret = poll(&fds, 1, 0);
if (ret < 0)
return;
@@ -359,11 +359,11 @@ protected:
int run()
{
- int slavefd = ipc_.create();
- if (slavefd < 0)
+ ScopedFD slavefd = ipc_.create();
+ if (!slavefd.isValid())
return TestFail;
- if (slaveStart(slavefd)) {
+ if (slaveStart(slavefd.release())) {
cerr << "Failed to start slave" << endl;
return TestFail;
}
IPCUnixSocket::create() creates two file descriptors. One of them is stored in IPCUnixSocket and the other is returned to a caller. This clarifies the ownership using ScopedFD. Signed-off-by: Hirokazu Honda <hiroh@chromium.org> --- include/libcamera/internal/ipc_unixsocket.h | 5 +-- src/libcamera/ipc_pipe_unixsocket.cpp | 8 ++--- src/libcamera/ipc_unixsocket.cpp | 40 ++++++++++----------- test/ipc/unixsocket.cpp | 6 ++-- 4 files changed, 30 insertions(+), 29 deletions(-)