[libcamera-devel] [PATCH v3] ipa: Use FileDescriptor instead of int in layers above IPC payload
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Thu Aug 19 09:13:40 CEST 2021
Hi Umang,
On Thu, Aug 19, 2021 at 11:35:13AM +0530, Umang Jain wrote:
> Hi Paul,
>
> On 8/17/21 10:16 AM, 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>
>
>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>
> Do you any preferences of merging this? I plan to merge the fd-leak test
I was just waiting for one more review before pushing it, thanks :)
> sometime today (so it'll fail / reproducing the issue), so this fix can be
> applied over it. I don't want a failing test lingering in master for too
> long, so I think merging this patch after the test is in, should be okay?
> Any thoughts? I can handle all the merging if you want that, as well :-)
Hiro's patch actually reminded me that I need to fix the deserializer
offset for FrameBuffer::plane, so I'll send a new version just so that
it can be tested first. I'll push it after that's approved.
Thanks,
Paul
>
>
> >
> > ---
> > 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
> > + */
> > + 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);
More information about the libcamera-devel
mailing list