NekoCWD/libcamera
NekoCWD/libcamera
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 7 Wiki Activity Actions

libcamera: ipc_unixsocket: Use UniqueFD for a file descriptor

Browse source 
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 UniqueFD.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
This commit is contained in:
Hirokazu Honda 2021-06-10 16:50:25 +09:00 • committed by Laurent Pinchart
parent edd70612e5
commit 9143668887
6 changed files with 49 additions and 44 deletions
Download patch file Download diff file Expand all files Collapse all files

7
include/libcamera/internal/ipc_unixsocket.h
View file

@ -12,6 +12,7 @@
#include <vector>
#include <libcamera/base/signal.h>
#include <libcamera/base/unique_fd.h>
namespace libcamera {
@ -28,8 +29,8 @@ public:
IPCUnixSocket();
~IPCUnixSocket();
int create();
int bind(int fd);
UniqueFD create();
int bind(UniqueFD fd);
void close();
bool isBound() const;
@ -49,7 +50,7 @@ private:
void dataNotifier();
int fd_;
UniqueFD fd_;
bool headerReceived_;
struct Header header_;
EventNotifier *notifier_;

8
src/libcamera/ipc_pipe_unixsocket.cpp
View file

@ -31,14 +31,14 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
args.push_back(ipaModulePath);
socket_ = std::make_unique<IPCUnixSocket>();
int fd = socket_->create();
if (fd < 0) {
UniqueFD 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);

43
src/libcamera/ipc_unixsocket.cpp
View file

@ -7,6 +7,7 @@
#include "libcamera/internal/ipc_unixsocket.h"
#include <array>
#include <poll.h>
#include <string.h>
#include <sys/socket.h>
@ -68,7 +69,7 @@ LOG_DEFINE_CATEGORY(IPCUnixSocket)
*/
IPCUnixSocket::IPCUnixSocket()
: fd_(-1), headerReceived_(false), notifier_(nullptr)
: headerReceived_(false), notifier_(nullptr)
{
}
@ -86,9 +87,9 @@ IPCUnixSocket::~IPCUnixSocket()
* to 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()
UniqueFD IPCUnixSocket::create()
{
int sockets[2];
int ret;
@ -98,14 +99,18 @@ int IPCUnixSocket::create()
ret = -errno;
LOG(IPCUnixSocket, Error)
<< "Failed to create socket pair: " << strerror(-ret);
return ret;
return {};
}
ret = bind(sockets[0]);
if (ret)
return ret;
std::array<UniqueFD, 2> socketFds{
UniqueFD(sockets[0]),
UniqueFD(sockets[1]),
};
return sockets[1];
if (bind(std::move(socketFds[0])) < 0)
return {};
return std::move(socketFds[1]);
}
/**
@ -118,13 +123,13 @@ int IPCUnixSocket::create()
*
* \return 0 on success or a negative error code otherwise
*/
int IPCUnixSocket::bind(int fd)
int IPCUnixSocket::bind(UniqueFD fd)
{
if (isBound())
return -EINVAL;
fd_ = fd;
notifier_ = new EventNotifier(fd_, EventNotifier::Read);
fd_ = std::move(fd);
notifier_ = new EventNotifier(fd_.get(), EventNotifier::Read);
notifier_->activated.connect(this, &IPCUnixSocket::dataNotifier);
return 0;
@ -143,9 +148,7 @@ void IPCUnixSocket::close()
delete notifier_;
notifier_ = nullptr;
::close(fd_);
fd_ = -1;
fd_.reset();
headerReceived_ = false;
}
@ -155,7 +158,7 @@ void IPCUnixSocket::close()
*/
bool IPCUnixSocket::isBound() const
{
return fd_ != -1;
return fd_.isValid();
}
/**
@ -182,7 +185,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)
@ -263,7 +266,7 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,
if (fds)
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);
@ -297,7 +300,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)
@ -317,7 +320,7 @@ void IPCUnixSocket::dataNotifier()
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)
@ -333,7 +336,7 @@ void IPCUnixSocket::dataNotifier()
* readyRead signal. The notifier will be reenabled by the receive()
* function.
*/
struct pollfd fds = { fd_, POLLIN, 0 };
struct pollfd fds = { fd_.get(), POLLIN, 0 };
ret = poll(&fds, 1, 0);
if (ret < 0)
return;

14
test/ipc/unixsocket.cpp
View file

@ -52,9 +52,9 @@ public:
ipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead);
}
int run(int fd)
int run(UniqueFD fd)
{
if (ipc_.bind(fd)) {
if (ipc_.bind(std::move(fd))) {
cerr << "Failed to connect to IPC channel" << endl;
return EXIT_FAILURE;
}
@ -359,11 +359,11 @@ protected:
int run()
{
int slavefd = ipc_.create();
if (slavefd < 0)
UniqueFD slavefd = ipc_.create();
if (!slavefd.isValid())
return TestFail;
if (slaveStart(slavefd)) {
if (slaveStart(slavefd.release())) {
cerr << "Failed to start slave" << endl;
return TestFail;
}
@ -495,9 +495,9 @@ private:
int main(int argc, char **argv)
{
if (argc == 2) {
int ipcfd = std::stoi(argv[1]);
UniqueFD ipcfd = UniqueFD(std::stoi(argv[1]));
UnixSocketTestSlave slave;
return slave.run(ipcfd);
return slave.run(std::move(ipcfd));
}
UnixSocketTest test;

8
test/ipc/unixsocket_ipc.cpp
View file

@ -49,9 +49,9 @@ public:
ipc_.readyRead.connect(this, &UnixSocketTestIPCSlave::readyRead);
}
int run(int fd)
int run(UniqueFD fd)
{
if (ipc_.bind(fd)) {
if (ipc_.bind(std::move(fd))) {
cerr << "Failed to connect to IPC channel" << endl;
return EXIT_FAILURE;
}
@ -222,9 +222,9 @@ int main(int argc, char **argv)
{
/* IPCPipeUnixSocket passes IPA module path in argv[1] */
if (argc == 3) {
int ipcfd = std::stoi(argv[2]);
UniqueFD ipcfd = UniqueFD(std::stoi(argv[2]));
UnixSocketTestIPCSlave slave;
return slave.run(ipcfd);
return slave.run(std::move(ipcfd));
}
UnixSocketTestIPC test;

13
utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
View file

@ -27,8 +27,9 @@
#include <libcamera/logging.h>
#include <libcamera/base/event_dispatcher.h>
#include <libcamera/base/thread.h>
#include <libcamera/base/log.h>
#include <libcamera/base/thread.h>
#include <libcamera/base/unique_fd.h>
#include "libcamera/internal/camera_sensor.h"
#include "libcamera/internal/control_serializer.h"
@ -122,9 +123,9 @@ public:
}
}
int init(std::unique_ptr<IPAModule> &ipam, int socketfd)
int init(std::unique_ptr<IPAModule> &ipam, UniqueFD socketfd)
{
if (socket_.bind(socketfd) < 0) {
if (socket_.bind(std::move(socketfd)) < 0) {
LOG({{proxy_worker_name}}, Error)
<< "IPC socket binding failed";
return EXIT_FAILURE;
@ -203,10 +204,10 @@ int main(int argc, char **argv)
return EXIT_FAILURE;
}
int fd = std::stoi(argv[2]);
UniqueFD fd(std::stoi(argv[2]));
LOG({{proxy_worker_name}}, Info)
<< "Starting worker for IPA module " << argv[1]
<< " with IPC fd = " << fd;
<< " with IPC fd = " << fd.get();
std::unique_ptr<IPAModule> ipam = std::make_unique<IPAModule>(argv[1]);
if (!ipam->isValid() || !ipam->load()) {
@ -228,7 +229,7 @@ int main(int argc, char **argv)
}
{{proxy_worker_name}} proxyWorker;
int ret = proxyWorker.init(ipam, fd);
int ret = proxyWorker.init(ipam, std::move(fd));
if (ret < 0) {
LOG({{proxy_worker_name}}, Error)
<< "Failed to initialize proxy worker";