[libcamera-devel] [PATCH v3] ipa: Use FileDescriptor instead of int in layers above IPC payload
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Aug 17 14:49:17 CEST 2021
Hi Paul,
Thank you for the patch.
On Tue, Aug 17, 2021 at 01:46:33PM +0900, Paul Elder wrote:
> Regarding (de)serialization in isolated IPA calls, we have four layers:
> - struct
> - byte vector + fd vector
> - IPCMessage
> - IPC payload
>
> The proxy handles the upper three layers (with help from the
> IPADataSerializer), and passes an IPCMessage to the IPC mechanism
> (implemented as an IPCPipe), which sends an IPC payload to its worker
> counterpart.
>
> When a FileDescriptor is involved, previously it was only a
> FileDescriptor in the first layer; in the lower three it was an int. To
> reduce the risk of potential fd leaks in the future, keep the
> FileDescriptor as-is throughout the upper three layers. Only the IPC
> mechanism will deal with ints, if it so wishes, when it does the actual
> IPC. IPCPipeUnixSocket does deal with ints for sending fds, so the
> conversion between IPCMessage and IPCUnixSocket::Payload converts
> between FileDescriptor and int.
>
> Additionally, change the data portion of the serialized form of
> FileDescriptor to a 32-bit unsigned integer, for alightnment purposes
> and in preparation for conversion to an index into the fd array.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v3:
> - encode isValid as a uint32
> ---
> .../libcamera/internal/ipa_data_serializer.h | 40 ++++-----
> include/libcamera/internal/ipc_pipe.h | 10 ++-
> src/libcamera/ipa_data_serializer.cpp | 86 +++++++++++--------
> src/libcamera/ipc_pipe.cpp | 12 ++-
> .../ipa_data_serializer_test.cpp | 12 +--
> .../module_ipa_proxy.cpp.tmpl | 2 +-
> .../module_ipa_proxy.h.tmpl | 2 +-
> .../libcamera_templates/proxy_functions.tmpl | 2 +-
> .../libcamera_templates/serializer.tmpl | 26 +++---
> 9 files changed, 105 insertions(+), 87 deletions(-)
>
> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
> index 519093bd..4353e07b 100644
> --- a/include/libcamera/internal/ipa_data_serializer.h
> +++ b/include/libcamera/internal/ipa_data_serializer.h
> @@ -66,7 +66,7 @@ template<typename T>
> class IPADataSerializer
> {
> public:
> - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> serialize(const T &data, ControlSerializer *cs = nullptr);
>
> static T deserialize(const std::vector<uint8_t> &data,
> @@ -76,12 +76,12 @@ public:
> ControlSerializer *cs = nullptr);
>
> static T deserialize(const std::vector<uint8_t> &data,
> - const std::vector<int32_t> &fds,
> + const std::vector<FileDescriptor> &fds,
> ControlSerializer *cs = nullptr);
> static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - std::vector<int32_t>::const_iterator fdsBegin,
> - std::vector<int32_t>::const_iterator fdsEnd,
> + std::vector<FileDescriptor>::const_iterator fdsBegin,
> + std::vector<FileDescriptor>::const_iterator fdsEnd,
> ControlSerializer *cs = nullptr);
> };
>
> @@ -104,11 +104,11 @@ template<typename V>
> class IPADataSerializer<std::vector<V>>
> {
> public:
> - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> serialize(const std::vector<V> &data, ControlSerializer *cs = nullptr)
> {
> std::vector<uint8_t> dataVec;
> - std::vector<int32_t> fdsVec;
> + std::vector<FileDescriptor> fdsVec;
>
> /* Serialize the length. */
> uint32_t vecLen = data.size();
> @@ -117,7 +117,7 @@ public:
> /* Serialize the members. */
> for (auto const &it : data) {
> std::vector<uint8_t> dvec;
> - std::vector<int32_t> fvec;
> + std::vector<FileDescriptor> fvec;
>
> std::tie(dvec, fvec) =
> IPADataSerializer<V>::serialize(it, cs);
> @@ -141,11 +141,11 @@ public:
> std::vector<uint8_t>::const_iterator dataEnd,
> ControlSerializer *cs = nullptr)
> {
> - std::vector<int32_t> fds;
> + std::vector<FileDescriptor> fds;
> return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);
> }
>
> - static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> + static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<FileDescriptor> &fds,
> ControlSerializer *cs = nullptr)
> {
> return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> @@ -153,15 +153,15 @@ public:
>
> static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - std::vector<int32_t>::const_iterator fdsBegin,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> + std::vector<FileDescriptor>::const_iterator fdsBegin,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd,
> ControlSerializer *cs = nullptr)
> {
> uint32_t vecLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> std::vector<V> ret(vecLen);
>
> std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;
> - std::vector<int32_t>::const_iterator fdIter = fdsBegin;
> + std::vector<FileDescriptor>::const_iterator fdIter = fdsBegin;
> for (uint32_t i = 0; i < vecLen; i++) {
> uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> uint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);
> @@ -201,11 +201,11 @@ template<typename K, typename V>
> class IPADataSerializer<std::map<K, V>>
> {
> public:
> - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> serialize(const std::map<K, V> &data, ControlSerializer *cs = nullptr)
> {
> std::vector<uint8_t> dataVec;
> - std::vector<int32_t> fdsVec;
> + std::vector<FileDescriptor> fdsVec;
>
> /* Serialize the length. */
> uint32_t mapLen = data.size();
> @@ -214,7 +214,7 @@ public:
> /* Serialize the members. */
> for (auto const &it : data) {
> std::vector<uint8_t> dvec;
> - std::vector<int32_t> fvec;
> + std::vector<FileDescriptor> fvec;
>
> std::tie(dvec, fvec) =
> IPADataSerializer<K>::serialize(it.first, cs);
> @@ -247,11 +247,11 @@ public:
> std::vector<uint8_t>::const_iterator dataEnd,
> ControlSerializer *cs = nullptr)
> {
> - std::vector<int32_t> fds;
> + std::vector<FileDescriptor> fds;
> return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);
> }
>
> - static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> + static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<FileDescriptor> &fds,
> ControlSerializer *cs = nullptr)
> {
> return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> @@ -259,8 +259,8 @@ public:
>
> static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - std::vector<int32_t>::const_iterator fdsBegin,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> + std::vector<FileDescriptor>::const_iterator fdsBegin,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd,
> ControlSerializer *cs = nullptr)
> {
> std::map<K, V> ret;
> @@ -268,7 +268,7 @@ public:
> uint32_t mapLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
>
> std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;
> - std::vector<int32_t>::const_iterator fdIter = fdsBegin;
> + std::vector<FileDescriptor>::const_iterator fdIter = fdsBegin;
> for (uint32_t i = 0; i < mapLen; i++) {
> uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> uint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);
> diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h
> index e58de340..52cc8fa3 100644
> --- a/include/libcamera/internal/ipc_pipe.h
> +++ b/include/libcamera/internal/ipc_pipe.h
> @@ -11,6 +11,8 @@
>
> #include <libcamera/base/signal.h>
>
> +#include <libcamera/file_descriptor.h>
> +
> #include "libcamera/internal/ipc_unixsocket.h"
>
> namespace libcamera {
> @@ -26,23 +28,23 @@ public:
> IPCMessage();
> IPCMessage(uint32_t cmd);
> IPCMessage(const Header &header);
> - IPCMessage(const IPCUnixSocket::Payload &payload);
> + IPCMessage(IPCUnixSocket::Payload &payload);
>
> IPCUnixSocket::Payload payload() const;
>
> Header &header() { return header_; }
> std::vector<uint8_t> &data() { return data_; }
> - std::vector<int32_t> &fds() { return fds_; }
> + std::vector<FileDescriptor> &fds() { return fds_; }
>
> const Header &header() const { return header_; }
> const std::vector<uint8_t> &data() const { return data_; }
> - const std::vector<int32_t> &fds() const { return fds_; }
> + const std::vector<FileDescriptor> &fds() const { return fds_; }
>
> private:
> Header header_;
>
> std::vector<uint8_t> data_;
> - std::vector<int32_t> fds_;
> + std::vector<FileDescriptor> fds_;
> };
>
> class IPCPipe
> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
> index fb941e6b..91471f52 100644
> --- a/src/libcamera/ipa_data_serializer.cpp
> +++ b/src/libcamera/ipa_data_serializer.cpp
> @@ -7,6 +7,8 @@
>
> #include "libcamera/internal/ipa_data_serializer.h"
>
> +#include <unistd.h>
> +
> #include <libcamera/base/log.h>
>
> /**
> @@ -141,7 +143,7 @@ namespace {
> /**
> * \fn template<typename T> IPADataSerializer<T>::deserialize(
> * const std::vector<uint8_t> &data,
> - * const std::vector<int32_t> &fds,
> + * const std::vector<FileDescriptor> &fds,
> * ControlSerializer *cs = nullptr)
> * \brief Deserialize byte vector and fd vector into an object
> * \tparam T Type of object to deserialize to
> @@ -162,8 +164,8 @@ namespace {
> * \fn template<typename T> IPADataSerializer::deserialize(
> * std::vector<uint8_t>::const_iterator dataBegin,
> * std::vector<uint8_t>::const_iterator dataEnd,
> - * std::vector<int32_t>::const_iterator fdsBegin,
> - * std::vector<int32_t>::const_iterator fdsEnd,
> + * std::vector<FileDescriptor>::const_iterator fdsBegin,
> + * std::vector<FileDescriptor>::const_iterator fdsEnd,
> * ControlSerializer *cs = nullptr)
> * \brief Deserialize byte vector and fd vector into an object
> * \tparam T Type of object to deserialize to
> @@ -187,7 +189,7 @@ namespace {
> #define DEFINE_POD_SERIALIZER(type) \
> \
> template<> \
> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> \
> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> \
> IPADataSerializer<type>::serialize(const type &data, \
> [[maybe_unused]] ControlSerializer *cs) \
> { \
> @@ -215,7 +217,7 @@ type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> \
> template<> \
> type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> - [[maybe_unused]] const std::vector<int32_t> &fds, \
> + [[maybe_unused]] const std::vector<FileDescriptor> &fds, \
> ControlSerializer *cs) \
> { \
> return deserialize(data.cbegin(), data.end(), cs); \
> @@ -224,8 +226,8 @@ type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> template<> \
> type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
> std::vector<uint8_t>::const_iterator dataEnd, \
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, \
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, \
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, \
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, \
> ControlSerializer *cs) \
> { \
> return deserialize(dataBegin, dataEnd, cs); \
> @@ -249,7 +251,7 @@ DEFINE_POD_SERIALIZER(double)
> * function parameter serdes).
> */
> template<>
> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> IPADataSerializer<std::string>::serialize(const std::string &data,
> [[maybe_unused]] ControlSerializer *cs)
> {
> @@ -276,7 +278,7 @@ IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator
> template<>
> std::string
> IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data,
> - [[maybe_unused]] const std::vector<int32_t> &fds,
> + [[maybe_unused]] const std::vector<FileDescriptor> &fds,
> [[maybe_unused]] ControlSerializer *cs)
> {
> return { data.cbegin(), data.cend() };
> @@ -286,8 +288,8 @@ template<>
> std::string
> IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd,
> [[maybe_unused]] ControlSerializer *cs)
> {
> return { dataBegin, dataEnd };
> @@ -305,7 +307,7 @@ IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator
> * be used. The serialized ControlInfoMap will have zero length.
> */
> template<>
> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> IPADataSerializer<ControlList>::serialize(const ControlList &data, ControlSerializer *cs)
> {
> if (!cs)
> @@ -405,7 +407,7 @@ IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data,
> template<>
> ControlList
> IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data,
> - [[maybe_unused]] const std::vector<int32_t> &fds,
> + [[maybe_unused]] const std::vector<FileDescriptor> &fds,
> ControlSerializer *cs)
> {
> return deserialize(data.cbegin(), data.end(), cs);
> @@ -415,8 +417,8 @@ template<>
> ControlList
> IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd,
> ControlSerializer *cs)
> {
> return deserialize(dataBegin, dataEnd, cs);
> @@ -429,7 +431,7 @@ IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator
> * X bytes - Serialized ControlInfoMap (using ControlSerializer)
> */
> template<>
> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> IPADataSerializer<ControlInfoMap>::serialize(const ControlInfoMap &map,
> ControlSerializer *cs)
> {
> @@ -491,7 +493,7 @@ IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data,
> template<>
> ControlInfoMap
> IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data,
> - [[maybe_unused]] const std::vector<int32_t> &fds,
> + [[maybe_unused]] const std::vector<FileDescriptor> &fds,
> ControlSerializer *cs)
> {
> return deserialize(data.cbegin(), data.end(), cs);
> @@ -501,8 +503,8 @@ template<>
> ControlInfoMap
> IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd,
> ControlSerializer *cs)
> {
> return deserialize(dataBegin, dataEnd, cs);
> @@ -522,37 +524,45 @@ IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_itera
> * 32-bit alignment of all serialized data
> */
> template<>
> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> IPADataSerializer<FileDescriptor>::serialize(const FileDescriptor &data,
> [[maybe_unused]] ControlSerializer *cs)
> {
> - std::vector<uint8_t> dataVec = { data.isValid() };
> - std::vector<int32_t> fdVec;
> + std::vector<uint8_t> dataVec;
> + std::vector<FileDescriptor> fdVec;
> +
> + /*
> + * Store as uint32_t to prepare for conversion from validity flag
> + * to index, and for alignment
s/alignment/alignment./
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + */
> + appendPOD<uint32_t>(dataVec, data.isValid());
> +
> if (data.isValid())
> - fdVec.push_back(data.fd());
> + fdVec.push_back(data);
> +
>
> return { dataVec, fdVec };
> }
>
> template<>
> -FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> - std::vector<uint8_t>::const_iterator dataEnd,
> - std::vector<int32_t>::const_iterator fdsBegin,
> - std::vector<int32_t>::const_iterator fdsEnd,
> +FileDescriptor IPADataSerializer<FileDescriptor>::deserialize([[maybe_unused]] std::vector<uint8_t>::const_iterator dataBegin,
> + [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,
> + std::vector<FileDescriptor>::const_iterator fdsBegin,
> + std::vector<FileDescriptor>::const_iterator fdsEnd,
> [[maybe_unused]] ControlSerializer *cs)
> {
> - ASSERT(std::distance(dataBegin, dataEnd) >= 1);
> + ASSERT(std::distance(dataBegin, dataEnd) >= 4);
>
> - bool valid = !!(*dataBegin);
> + uint32_t valid = readPOD<uint32_t>(dataBegin, 0, dataEnd);
>
> ASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));
>
> - return valid ? FileDescriptor(*fdsBegin) : FileDescriptor();
> + return valid ? *fdsBegin : FileDescriptor();
> }
>
> template<>
> FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(const std::vector<uint8_t> &data,
> - const std::vector<int32_t> &fds,
> + const std::vector<FileDescriptor> &fds,
> [[maybe_unused]] ControlSerializer *cs)
> {
> return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end());
> @@ -565,15 +575,15 @@ FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(const std::vector<
> * 4 bytes - uint32_t Length
> */
> template<>
> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> IPADataSerializer<FrameBuffer::Plane>::serialize(const FrameBuffer::Plane &data,
> [[maybe_unused]] ControlSerializer *cs)
> {
> std::vector<uint8_t> dataVec;
> - std::vector<int32_t> fdsVec;
> + std::vector<FileDescriptor> fdsVec;
>
> std::vector<uint8_t> fdBuf;
> - std::vector<int32_t> fdFds;
> + std::vector<FileDescriptor> fdFds;
> std::tie(fdBuf, fdFds) =
> IPADataSerializer<FileDescriptor>::serialize(data.fd);
> dataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end());
> @@ -588,13 +598,13 @@ template<>
> FrameBuffer::Plane
> IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - std::vector<int32_t>::const_iterator fdsBegin,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> + std::vector<FileDescriptor>::const_iterator fdsBegin,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd,
> [[maybe_unused]] ControlSerializer *cs)
> {
> FrameBuffer::Plane ret;
>
> - ret.fd = IPADataSerializer<FileDescriptor>::deserialize(dataBegin, dataBegin + 1,
> + ret.fd = IPADataSerializer<FileDescriptor>::deserialize(dataBegin, dataBegin + 4,
> fdsBegin, fdsBegin + 1);
> ret.length = readPOD<uint32_t>(dataBegin, 1, dataEnd);
>
> @@ -604,7 +614,7 @@ IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_i
> template<>
> FrameBuffer::Plane
> IPADataSerializer<FrameBuffer::Plane>::deserialize(const std::vector<uint8_t> &data,
> - const std::vector<int32_t> &fds,
> + const std::vector<FileDescriptor> &fds,
> ControlSerializer *cs)
> {
> return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp
> index 28e20e03..84136a82 100644
> --- a/src/libcamera/ipc_pipe.cpp
> +++ b/src/libcamera/ipc_pipe.cpp
> @@ -77,13 +77,17 @@ IPCMessage::IPCMessage(const Header &header)
> *
> * This essentially converts an IPCUnixSocket payload into an IPCMessage.
> * The header is extracted from the payload into the IPCMessage's header field.
> + *
> + * If the IPCUnixSocket payload had any valid file descriptors, then they will
> + * all be invalidated.
> */
> -IPCMessage::IPCMessage(const IPCUnixSocket::Payload &payload)
> +IPCMessage::IPCMessage(IPCUnixSocket::Payload &payload)
> {
> memcpy(&header_, payload.data.data(), sizeof(header_));
> data_ = std::vector<uint8_t>(payload.data.begin() + sizeof(header_),
> payload.data.end());
> - fds_ = payload.fds;
> + for (int32_t &fd : payload.fds)
> + fds_.push_back(FileDescriptor(std::move(fd)));
> }
>
> /**
> @@ -104,7 +108,9 @@ IPCUnixSocket::Payload IPCMessage::payload() const
>
> /* \todo Make this work without copy */
> memcpy(payload.data.data() + sizeof(Header), data_.data(), data_.size());
> - payload.fds = fds_;
> +
> + for (const FileDescriptor &fd : fds_)
> + payload.fds.push_back(fd.fd());
>
> return payload;
> }
> diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp
> index bf7e34e2..eca19a66 100644
> --- a/test/serialization/ipa_data_serializer_test.cpp
> +++ b/test/serialization/ipa_data_serializer_test.cpp
> @@ -53,7 +53,7 @@ template<typename T>
> int testPodSerdes(T in)
> {
> std::vector<uint8_t> buf;
> - std::vector<int32_t> fds;
> + std::vector<FileDescriptor> fds;
>
> std::tie(buf, fds) = IPADataSerializer<T>::serialize(in);
> T out = IPADataSerializer<T>::deserialize(buf, fds);
> @@ -72,7 +72,7 @@ int testVectorSerdes(const std::vector<T> &in,
> ControlSerializer *cs = nullptr)
> {
> std::vector<uint8_t> buf;
> - std::vector<int32_t> fds;
> + std::vector<FileDescriptor> fds;
>
> std::tie(buf, fds) = IPADataSerializer<std::vector<T>>::serialize(in, cs);
> std::vector<T> out = IPADataSerializer<std::vector<T>>::deserialize(buf, fds, cs);
> @@ -92,7 +92,7 @@ int testMapSerdes(const std::map<K, V> &in,
> ControlSerializer *cs = nullptr)
> {
> std::vector<uint8_t> buf;
> - std::vector<int32_t> fds;
> + std::vector<FileDescriptor> fds;
>
> std::tie(buf, fds) = IPADataSerializer<std::map<K, V>>::serialize(in, cs);
> std::map<K, V> out = IPADataSerializer<std::map<K, V>>::deserialize(buf, fds, cs);
> @@ -219,7 +219,7 @@ private:
> };
>
> std::vector<uint8_t> buf;
> - std::vector<int32_t> fds;
> + std::vector<FileDescriptor> fds;
>
> if (testVectorSerdes(vecUint8) != TestPass)
> return TestFail;
> @@ -291,7 +291,7 @@ private:
> { { "a", { 1, 2, 3 } }, { "b", { 4, 5, 6 } }, { "c", { 7, 8, 9 } } };
>
> std::vector<uint8_t> buf;
> - std::vector<int32_t> fds;
> + std::vector<FileDescriptor> fds;
>
> if (testMapSerdes(mapUintStr) != TestPass)
> return TestFail;
> @@ -359,7 +359,7 @@ private:
> std::string strEmpty = "";
>
> std::vector<uint8_t> buf;
> - std::vector<int32_t> fds;
> + std::vector<FileDescriptor> fds;
>
> if (testPodSerdes(u32min) != TestPass)
> return TestFail;
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> index a4e008c7..aab1fffb 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> @@ -236,7 +236,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> void {{proxy_name}}::{{method.mojom_name}}IPC(
> std::vector<uint8_t>::const_iterator data,
> size_t dataSize,
> - [[maybe_unused]] const std::vector<int32_t> &fds)
> + [[maybe_unused]] const std::vector<FileDescriptor> &fds)
> {
> {%- for param in method.parameters %}
> {{param|name}} {{param.mojom_name}};
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> index c222f5f2..1979e68f 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> @@ -65,7 +65,7 @@ private:
> void {{method.mojom_name}}IPC(
> std::vector<uint8_t>::const_iterator data,
> size_t dataSize,
> - const std::vector<int32_t> &fds);
> + const std::vector<FileDescriptor> &fds);
> {% endfor %}
>
> /* Helper class to invoke async functions in another thread. */
> diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> index ea9cc86b..ebcd2aaa 100644
> --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> @@ -54,7 +54,7 @@
> {%- for param in params %}
> std::vector<uint8_t> {{param.mojom_name}}Buf;
> {%- if param|has_fd %}
> - std::vector<int32_t> {{param.mojom_name}}Fds;
> + std::vector<FileDescriptor> {{param.mojom_name}}Fds;
> std::tie({{param.mojom_name}}Buf, {{param.mojom_name}}Fds) =
> {%- else %}
> std::tie({{param.mojom_name}}Buf, std::ignore) =
> diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> index d8d55807..b8ef8e7b 100644
> --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> @@ -40,7 +40,7 @@
> retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
> {%- elif field|is_fd %}
> std::vector<uint8_t> {{field.mojom_name}};
> - std::vector<int32_t> {{field.mojom_name}}Fds;
> + std::vector<FileDescriptor> {{field.mojom_name}}Fds;
> std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) =
> IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});
> retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
> @@ -58,7 +58,7 @@
> {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}
> std::vector<uint8_t> {{field.mojom_name}};
> {%- if field|has_fd %}
> - std::vector<int32_t> {{field.mojom_name}}Fds;
> + std::vector<FileDescriptor> {{field.mojom_name}}Fds;
> std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) =
> {%- else %}
> std::tie({{field.mojom_name}}, std::ignore) =
> @@ -104,9 +104,9 @@
> dataSize -= {{field_size}};
> {%- endif %}
> {% elif field|is_fd %}
> - {%- set field_size = 1 %}
> + {%- set field_size = 4 %}
> {{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
> - ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs);
> + ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs);
> {%- if not loop.last %}
> m += {{field_size}};
> dataSize -= {{field_size}};
> @@ -177,7 +177,7 @@
> # \a struct.
> #}
> {%- macro serializer(struct, namespace) %}
> - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>>
> serialize(const {{struct|name_full}} &data,
> {%- if struct|needs_control_serializer %}
> ControlSerializer *cs)
> @@ -187,7 +187,7 @@
> {
> std::vector<uint8_t> retData;
> {%- if struct|has_fd %}
> - std::vector<int32_t> retFds;
> + std::vector<FileDescriptor> retFds;
> {%- endif %}
> {%- for field in struct.fields %}
> {{serializer_field(field, namespace, loop)}}
> @@ -210,7 +210,7 @@
> {%- macro deserializer_fd(struct, namespace) %}
> static {{struct|name_full}}
> deserialize(std::vector<uint8_t> &data,
> - std::vector<int32_t> &fds,
> + std::vector<FileDescriptor> &fds,
> {%- if struct|needs_control_serializer %}
> ControlSerializer *cs)
> {%- else %}
> @@ -224,8 +224,8 @@
> static {{struct|name_full}}
> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - std::vector<int32_t>::const_iterator fdsBegin,
> - std::vector<int32_t>::const_iterator fdsEnd,
> + std::vector<FileDescriptor>::const_iterator fdsBegin,
> + std::vector<FileDescriptor>::const_iterator fdsEnd,
> {%- if struct|needs_control_serializer %}
> ControlSerializer *cs)
> {%- else %}
> @@ -234,7 +234,7 @@
> {
> {{struct|name_full}} ret;
> std::vector<uint8_t>::const_iterator m = dataBegin;
> - std::vector<int32_t>::const_iterator n = fdsBegin;
> + std::vector<FileDescriptor>::const_iterator n = fdsBegin;
>
> size_t dataSize = std::distance(dataBegin, dataEnd);
> [[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd);
> @@ -255,7 +255,7 @@
> {%- macro deserializer_fd_simple(struct, namespace) %}
> static {{struct|name_full}}
> deserialize(std::vector<uint8_t> &data,
> - [[maybe_unused]] std::vector<int32_t> &fds,
> + [[maybe_unused]] std::vector<FileDescriptor> &fds,
> ControlSerializer *cs = nullptr)
> {
> return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);
> @@ -264,8 +264,8 @@
> static {{struct|name_full}}
> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> std::vector<uint8_t>::const_iterator dataEnd,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin,
> + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd,
> ControlSerializer *cs = nullptr)
> {
> return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list