[{"id":18249,"web_url":"https://patchwork.libcamera.org/comment/18249/","msgid":"","date":"2021-07-21T13:35:45","subject":"Re: [libcamera-devel] [PATCH v2] ipa: Use FileDescriptor instead of\n\tint in layers above IPC payload","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Jul 21, 2021 at 07:29:52PM +0900, Paul Elder wrote:\n> Regarding (de)serialization in isolated IPA calls, we have four layers:\n> - struct\n> - byte vector + fd vector\n> - IPCMessage\n> - IPC payload\n> \n> The proxy handles the upper three layers (with help from the\n> IPADataSerializer), and passes an IPCMessage to the IPC mechanism\n> (implemented as an IPCPipe), which sends an IPC payload to its worker\n> counterpart.\n> \n> When a FileDescriptor is involved, previously it was only a\n> FileDescriptor in the first layer; in the lower three it was an int. To\n> reduce the risk of potential fd leaks in the future, keep the\n> FileDescriptor as-is throughout the upper three layers. Only the IPC\n> mechanism will deal with ints, if it so wishes, when it does the actual\n> IPC. IPCPipeUnixSocket does deal with ints for sending fds, so the\n> conversion between IPCMessage and IPCUnixSocket::Payload converts\n> between FileDescriptor and int.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Umang, could you please check if this fixes the issue?\n> ---\n> .../libcamera/internal/ipa_data_serializer.h | 40 +++++-----\n> include/libcamera/internal/ipc_pipe.h | 10 ++-\n> src/libcamera/ipa_data_serializer.cpp | 78 +++++++++----------\n> src/libcamera/ipc_pipe.cpp | 12 ++-\n> .../ipa_data_serializer_test.cpp | 12 +--\n> .../module_ipa_proxy.cpp.tmpl | 2 +-\n> .../module_ipa_proxy.h.tmpl | 2 +-\n> .../libcamera_templates/proxy_functions.tmpl | 2 +-\n> .../libcamera_templates/serializer.tmpl | 22 +++---\n> 9 files changed, 90 insertions(+), 90 deletions(-)\n> \n> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h\n> index 519093bd..4353e07b 100644\n> --- a/include/libcamera/internal/ipa_data_serializer.h\n> +++ b/include/libcamera/internal/ipa_data_serializer.h\n> @@ -66,7 +66,7 @@ template\n> class IPADataSerializer\n> {\n> public:\n> -\tstatic std::tuple, std::vector>\n> +\tstatic std::tuple, std::vector>\n> \tserialize(const T &data, ControlSerializer *cs = nullptr);\n> \n> \tstatic T deserialize(const std::vector &data,\n> @@ -76,12 +76,12 @@ public:\n> \t\t\t ControlSerializer *cs = nullptr);\n> \n> \tstatic T deserialize(const std::vector &data,\n> -\t\t\t const std::vector &fds,\n> +\t\t\t const std::vector &fds,\n> \t\t\t ControlSerializer *cs = nullptr);\n> \tstatic T deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t std::vector::const_iterator fdsEnd,\n> +\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t std::vector::const_iterator fdsEnd,\n\nWe could make the iterators non-const to allow moving FileDescriptor\ninstances (as was initially envisioned to fix the fd leak, by moving the\nints to use the FileDescriptor(int &&fd) constructor), but copying a\nFileDescriptor is quite cheap due to the implicit sharing of the\ninternal data, so it's not a must. In any case, I don't think it should\nbe done as part of this patch.\n\n> \t\t\t ControlSerializer *cs = nullptr);\n> };\n> \n> @@ -104,11 +104,11 @@ template\n> class IPADataSerializer>\n> {\n> public:\n> -\tstatic std::tuple, std::vector>\n> +\tstatic std::tuple, std::vector>\n> \tserialize(const std::vector &data, ControlSerializer *cs = nullptr)\n> \t{\n> \t\tstd::vector dataVec;\n> -\t\tstd::vector fdsVec;\n> +\t\tstd::vector fdsVec;\n> \n> \t\t/* Serialize the length. */\n> \t\tuint32_t vecLen = data.size();\n> @@ -117,7 +117,7 @@ public:\n> \t\t/* Serialize the members. */\n> \t\tfor (auto const &it : data) {\n> \t\t\tstd::vector dvec;\n> -\t\t\tstd::vector fvec;\n> +\t\t\tstd::vector fvec;\n> \n> \t\t\tstd::tie(dvec, fvec) =\n> \t\t\t\tIPADataSerializer::serialize(it, cs);\n> @@ -141,11 +141,11 @@ public:\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \t\treturn deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);\n> \t}\n> \n> -\tstatic std::vector deserialize(std::vector &data, std::vector &fds,\n> +\tstatic std::vector deserialize(std::vector &data, std::vector &fds,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);\n> @@ -153,15 +153,15 @@ public:\n> \n> \tstatic std::vector deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\tuint32_t vecLen = readPOD(dataBegin, 0, dataEnd);\n> \t\tstd::vector ret(vecLen);\n> \n> \t\tstd::vector::const_iterator dataIter = dataBegin + 4;\n> -\t\tstd::vector::const_iterator fdIter = fdsBegin;\n> +\t\tstd::vector::const_iterator fdIter = fdsBegin;\n> \t\tfor (uint32_t i = 0; i < vecLen; i++) {\n> \t\t\tuint32_t sizeofData = readPOD(dataIter, 0, dataEnd);\n> \t\t\tuint32_t sizeofFds = readPOD(dataIter, 4, dataEnd);\n> @@ -201,11 +201,11 @@ template\n> class IPADataSerializer>\n> {\n> public:\n> -\tstatic std::tuple, std::vector>\n> +\tstatic std::tuple, std::vector>\n> \tserialize(const std::map &data, ControlSerializer *cs = nullptr)\n> \t{\n> \t\tstd::vector dataVec;\n> -\t\tstd::vector fdsVec;\n> +\t\tstd::vector fdsVec;\n> \n> \t\t/* Serialize the length. */\n> \t\tuint32_t mapLen = data.size();\n> @@ -214,7 +214,7 @@ public:\n> \t\t/* Serialize the members. */\n> \t\tfor (auto const &it : data) {\n> \t\t\tstd::vector dvec;\n> -\t\t\tstd::vector fvec;\n> +\t\t\tstd::vector fvec;\n> \n> \t\t\tstd::tie(dvec, fvec) =\n> \t\t\t\tIPADataSerializer::serialize(it.first, cs);\n> @@ -247,11 +247,11 @@ public:\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \t\treturn deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);\n> \t}\n> \n> -\tstatic std::map deserialize(std::vector &data, std::vector &fds,\n> +\tstatic std::map deserialize(std::vector &data, std::vector &fds,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);\n> @@ -259,8 +259,8 @@ public:\n> \n> \tstatic std::map deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\tstd::map ret;\n> @@ -268,7 +268,7 @@ public:\n> \t\tuint32_t mapLen = readPOD(dataBegin, 0, dataEnd);\n> \n> \t\tstd::vector::const_iterator dataIter = dataBegin + 4;\n> -\t\tstd::vector::const_iterator fdIter = fdsBegin;\n> +\t\tstd::vector::const_iterator fdIter = fdsBegin;\n> \t\tfor (uint32_t i = 0; i < mapLen; i++) {\n> \t\t\tuint32_t sizeofData = readPOD(dataIter, 0, dataEnd);\n> \t\t\tuint32_t sizeofFds = readPOD(dataIter, 4, dataEnd);\n> diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h\n> index e58de340..52cc8fa3 100644\n> --- a/include/libcamera/internal/ipc_pipe.h\n> +++ b/include/libcamera/internal/ipc_pipe.h\n> @@ -11,6 +11,8 @@\n> \n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/ipc_unixsocket.h\"\n> \n> namespace libcamera {\n> @@ -26,23 +28,23 @@ public:\n> \tIPCMessage();\n> \tIPCMessage(uint32_t cmd);\n> \tIPCMessage(const Header &header);\n> -\tIPCMessage(const IPCUnixSocket::Payload &payload);\n> +\tIPCMessage(IPCUnixSocket::Payload &payload);\n> \n> \tIPCUnixSocket::Payload payload() const;\n> \n> \tHeader &header() { return header_; }\n> \tstd::vector &data() { return data_; }\n> -\tstd::vector &fds() { return fds_; }\n> +\tstd::vector &fds() { return fds_; }\n> \n> \tconst Header &header() const { return header_; }\n> \tconst std::vector &data() const { return data_; }\n> -\tconst std::vector &fds() const { return fds_; }\n> +\tconst std::vector &fds() const { return fds_; }\n> \n> private:\n> \tHeader header_;\n> \n> \tstd::vector data_;\n> -\tstd::vector fds_;\n> +\tstd::vector fds_;\n> };\n> \n> class IPCPipe\n> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n> index fb941e6b..e991d751 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -8,6 +8,7 @@\n> #include \"libcamera/internal/ipa_data_serializer.h\"\n> \n> #include \n> +#include \n\nThis should go in a separate group above log.h.\n\n> \n> /**\n> * \\file ipa_data_serializer.h\n> @@ -141,7 +142,7 @@ namespace {\n> /**\n> * \\fn template IPADataSerializer::deserialize(\n> * \tconst std::vector &data,\n> - * \tconst std::vector &fds,\n> + * \tconst std::vector &fds,\n> * \tControlSerializer *cs = nullptr)\n> * \\brief Deserialize byte vector and fd vector into an object\n> * \\tparam T Type of object to deserialize to\n> @@ -162,8 +163,8 @@ namespace {\n> * \\fn template IPADataSerializer::deserialize(\n> * \tstd::vector::const_iterator dataBegin,\n> * \tstd::vector::const_iterator dataEnd,\n> - * \tstd::vector::const_iterator fdsBegin,\n> - * \tstd::vector::const_iterator fdsEnd,\n> + * \tstd::vector::const_iterator fdsBegin,\n> + * \tstd::vector::const_iterator fdsEnd,\n> * \tControlSerializer *cs = nullptr)\n> * \\brief Deserialize byte vector and fd vector into an object\n> * \\tparam T Type of object to deserialize to\n> @@ -187,7 +188,7 @@ namespace {\n> #define DEFINE_POD_SERIALIZER(type)\t\t\t\t\t\\\n> \t\t\t\t\t\t\t\t\t\\\n> template<>\t\t\t\t\t\t\t\t\\\n> -std::tuple, std::vector>\t\t\t\\\n> +std::tuple, std::vector>\t\t\t\\\n\nThere's now one tab too much before the \\.\n\n> IPADataSerializer::serialize(const type &data,\t\t\t\\\n> \t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> {\t\t\t\t\t\t\t\t\t\\\n> @@ -215,7 +216,7 @@ type IPADataSerializer::deserialize(const std::vector &data, \\\n> \t\t\t\t\t\t\t\t\t\\\n> template<>\t\t\t\t\t\t\t\t\\\n> type IPADataSerializer::deserialize(const std::vector &data, \\\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n> +\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n> \t\t\t\t\t ControlSerializer *cs)\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> \treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n> @@ -224,8 +225,8 @@ type IPADataSerializer::deserialize(const std::vector &data, \\\n> template<>\t\t\t\t\t\t\t\t\\\n> type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> \t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n> \t\t\t\t\t ControlSerializer *cs)\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> \treturn deserialize(dataBegin, dataEnd, cs);\t\t\t\\\n> @@ -249,7 +250,7 @@ DEFINE_POD_SERIALIZER(double)\n> * function parameter serdes).\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const std::string &data,\n> \t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> @@ -276,7 +277,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator\n> template<>\n> std::string\n> IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> +\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> \t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \treturn { data.cbegin(), data.cend() };\n> @@ -286,8 +287,8 @@ template<>\n> std::string\n> IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \treturn { dataBegin, dataEnd };\n> @@ -305,7 +306,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator\n> * be used. The serialized ControlInfoMap will have zero length.\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const ControlList &data, ControlSerializer *cs)\n> {\n> \tif (!cs)\n> @@ -405,7 +406,7 @@ IPADataSerializer::deserialize(const std::vector &data,\n> template<>\n> ControlList\n> IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> +\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(data.cbegin(), data.end(), cs);\n> @@ -415,8 +416,8 @@ template<>\n> ControlList\n> IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(dataBegin, dataEnd, cs);\n> @@ -429,7 +430,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator\n> * X bytes - Serialized ControlInfoMap (using ControlSerializer)\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const ControlInfoMap &map,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> @@ -491,7 +492,7 @@ IPADataSerializer::deserialize(const std::vector &data,\n> template<>\n> ControlInfoMap\n> IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> +\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(data.cbegin(), data.end(), cs);\n> @@ -501,8 +502,8 @@ template<>\n> ControlInfoMap\n> IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(dataBegin, dataEnd, cs);\n> @@ -522,37 +523,28 @@ IPADataSerializer::deserialize(std::vector::const_itera\n> * 32-bit alignment of all serialized data\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const FileDescriptor &data,\n> \t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> -\tstd::vector dataVec = { data.isValid() };\n> -\tstd::vector fdVec;\n> -\tif (data.isValid())\n> -\t\tfdVec.push_back(data.fd());\n> -\n> -\treturn { dataVec, fdVec };\n> +\treturn { {}, { data } };\n\nAs discussed on IRC, moving forward, I think we should serialize file\ndescriptors with an entry in the data array that would point (as an\noffset) to the FileDescriptor entry in the fds vector. This would help\nnot making the wire protocol dependent on the serializer implementation.\nWe can't do so right now as the serialize() function doesn't have access\nto the necessary information, so dropping the isValid() byte should be\nfine for now. Could you however file a bug to record this ?\n\n> }\n> \n> template<>\n> -FileDescriptor IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> -\t\t\t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t\t\t std::vector::const_iterator fdsEnd,\n> +FileDescriptor IPADataSerializer::deserialize([[maybe_unused]] std::vector::const_iterator dataBegin,\n> +\t\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator dataEnd,\n> +\t\t\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t\t\t std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> -\tASSERT(std::distance(dataBegin, dataEnd) >= 1);\n> -\n> -\tbool valid = !!(*dataBegin);\n> -\n> -\tASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));\n> +\tASSERT(std::distance(fdsBegin, fdsEnd) == 1);\n\nI don't recall if deserialize() is called with [begin, end[ covering\nonly the fd entries needed to deserialize the type T, or with all the\nremaining entries. In the latter case (which seems to be implied by the\nexisting code checking for >= 1), this should be\n\n\tASSERT(std::distance(fdsBegin, fdsEnd) >= 1);\n\n> \n> -\treturn valid ? FileDescriptor(*fdsBegin) : FileDescriptor();\n> +\treturn *fdsBegin;\n> }\n> \n> template<>\n> FileDescriptor IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t\t\t const std::vector &fds,\n> +\t\t\t\t\t\t\t const std::vector &fds,\n> \t\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end());\n> @@ -565,15 +557,15 @@ FileDescriptor IPADataSerializer::deserialize(const std::vector<\n> * 4 bytes - uint32_t Length\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const FrameBuffer::Plane &data,\n> \t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \tstd::vector dataVec;\n> -\tstd::vector fdsVec;\n> +\tstd::vector fdsVec;\n> \n> \tstd::vector fdBuf;\n> -\tstd::vector fdFds;\n> +\tstd::vector fdFds;\n> \tstd::tie(fdBuf, fdFds) =\n> \t\tIPADataSerializer::serialize(data.fd);\n> \tdataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end());\n> @@ -588,8 +580,8 @@ template<>\n> FrameBuffer::Plane\n> IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \tFrameBuffer::Plane ret;\n> @@ -604,7 +596,7 @@ IPADataSerializer::deserialize(std::vector::const_i\n> template<>\n> FrameBuffer::Plane\n> IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t\t const std::vector &fds,\n> +\t\t\t\t\t\t const std::vector &fds,\n> \t\t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);\n> diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp\n> index 28e20e03..84136a82 100644\n> --- a/src/libcamera/ipc_pipe.cpp\n> +++ b/src/libcamera/ipc_pipe.cpp\n> @@ -77,13 +77,17 @@ IPCMessage::IPCMessage(const Header &header)\n> *\n> * This essentially converts an IPCUnixSocket payload into an IPCMessage.\n> * The header is extracted from the payload into the IPCMessage's header field.\n> + *\n> + * If the IPCUnixSocket payload had any valid file descriptors, then they will\n> + * all be invalidated.\n> */\n> -IPCMessage::IPCMessage(const IPCUnixSocket::Payload &payload)\n> +IPCMessage::IPCMessage(IPCUnixSocket::Payload &payload)\n> {\n> \tmemcpy(&header_, payload.data.data(), sizeof(header_));\n> \tdata_ = std::vector(payload.data.begin() + sizeof(header_),\n> \t\t\t\t payload.data.end());\n> -\tfds_ = payload.fds;\n> +\tfor (int32_t &fd : payload.fds)\n> +\t\tfds_.push_back(FileDescriptor(std::move(fd)));\n> }\n> \n> /**\n> @@ -104,7 +108,9 @@ IPCUnixSocket::Payload IPCMessage::payload() const\n> \n> \t/* \\todo Make this work without copy */\n> \tmemcpy(payload.data.data() + sizeof(Header), data_.data(), data_.size());\n> -\tpayload.fds = fds_;\n> +\n> +\tfor (const FileDescriptor &fd : fds_)\n> +\t\tpayload.fds.push_back(fd.fd());\n\nThis is the tricky part, where the invariants are broken. If one were to\nwrite\n\n\tIPCMessage message = ...;\n\n\tIPCUnixSocket::Payload payload = message.payload();\n\tIPCMessage otherMessage{ payload };\n\nthen we would have two messages that contain file descriptor instances\nreferring to the same numerical fd, without knowing about each other. We\nwould also have issues with\n\n\tIPCUnixSocket::Payload payload;\n\n\t{\n\t\tIPCMessage message = ...;\n\t\tpayload = message.payload();\n\t}\n\n\tsend(payload);\n\nas the file descriptors in the payload would get closed when message is\ndestroyed.\n\nI expect the lower IPC layers to be reworked in the future, and as\npayloads are only used in a small number of places where they're\ndirectly passed to a send() function while the IPCMessage is still\nvalid, I think we can live with this issue for the time being. It would\nhowever be good to document it, stating in the documentation of\nIPCMessage::payload() that the returned payload is only valid during the\nlifetime of the IPCMessage, and with a \\todo comment in the function to\nexplain the problems.\n\n> \n> \treturn payload;\n> }\n> diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp\n> index 880bcd02..1e8f7e21 100644\n> --- a/test/serialization/ipa_data_serializer_test.cpp\n> +++ b/test/serialization/ipa_data_serializer_test.cpp\n> @@ -53,7 +53,7 @@ template\n> int testPodSerdes(T in)\n> {\n> \tstd::vector buf;\n> -\tstd::vector fds;\n> +\tstd::vector fds;\n> \n> \tstd::tie(buf, fds) = IPADataSerializer::serialize(in);\n> \tT out = IPADataSerializer::deserialize(buf, fds);\n> @@ -72,7 +72,7 @@ int testVectorSerdes(const std::vector &in,\n> \t\t ControlSerializer *cs = nullptr)\n> {\n> \tstd::vector buf;\n> -\tstd::vector fds;\n> +\tstd::vector fds;\n> \n> \tstd::tie(buf, fds) = IPADataSerializer>::serialize(in, cs);\n> \tstd::vector out = IPADataSerializer>::deserialize(buf, fds, cs);\n> @@ -92,7 +92,7 @@ int testMapSerdes(const std::map &in,\n> \t\t ControlSerializer *cs = nullptr)\n> {\n> \tstd::vector buf;\n> -\tstd::vector fds;\n> +\tstd::vector fds;\n> \n> \tstd::tie(buf, fds) = IPADataSerializer>::serialize(in, cs);\n> \tstd::map out = IPADataSerializer>::deserialize(buf, fds, cs);\n> @@ -219,7 +219,7 @@ private:\n> \t\t};\n> \n> \t\tstd::vector buf;\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \n> \t\tif (testVectorSerdes(vecUint8) != TestPass)\n> \t\t\treturn TestFail;\n> @@ -291,7 +291,7 @@ private:\n> \t\t\t{ { \"a\", { 1, 2, 3 } }, { \"b\", { 4, 5, 6 } }, { \"c\", { 7, 8, 9 } } };\n> \n> \t\tstd::vector buf;\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \n> \t\tif (testMapSerdes(mapUintStr) != TestPass)\n> \t\t\treturn TestFail;\n> @@ -359,7 +359,7 @@ private:\n> \t\tstd::string strEmpty = \"\";\n> \n> \t\tstd::vector buf;\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \n> \t\tif (testPodSerdes(u32min) != TestPass)\n> \t\t\treturn TestFail;\n> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> index a4e008c7..aab1fffb 100644\n> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> @@ -236,7 +236,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)\n> void {{proxy_name}}::{{method.mojom_name}}IPC(\n> \tstd::vector::const_iterator data,\n> \tsize_t dataSize,\n> -\t[[maybe_unused]] const std::vector &fds)\n> +\t[[maybe_unused]] const std::vector &fds)\n> {\n> {%- for param in method.parameters %}\n> \t{{param|name}} {{param.mojom_name}};\n> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> index ae168548..09e3af7d 100644\n> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> @@ -65,7 +65,7 @@ private:\n> \tvoid {{method.mojom_name}}IPC(\n> \t\tstd::vector::const_iterator data,\n> \t\tsize_t dataSize,\n> -\t\tconst std::vector &fds);\n> +\t\tconst std::vector &fds);\n> {% endfor %}\n> \n> \t/* Helper class to invoke async functions in another thread. */\n> diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> index ea9cc86b..ebcd2aaa 100644\n> --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> @@ -54,7 +54,7 @@\n> {%- for param in params %}\n> \tstd::vector {{param.mojom_name}}Buf;\n> {%- if param|has_fd %}\n> -\tstd::vector {{param.mojom_name}}Fds;\n> +\tstd::vector {{param.mojom_name}}Fds;\n> \tstd::tie({{param.mojom_name}}Buf, {{param.mojom_name}}Fds) =\n> {%- else %}\n> \tstd::tie({{param.mojom_name}}Buf, std::ignore) =\n> diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> index d8d55807..cdd31df8 100644\n> --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> @@ -40,7 +40,7 @@\n> \t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n> {%- elif field|is_fd %}\n> \t\tstd::vector {{field.mojom_name}};\n> -\t\tstd::vector {{field.mojom_name}}Fds;\n> +\t\tstd::vector {{field.mojom_name}}Fds;\n> \t\tstd::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) =\n> \t\t\tIPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});\n> \t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n> @@ -58,7 +58,7 @@\n> {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}\n> \t\tstd::vector {{field.mojom_name}};\n> \t{%- if field|has_fd %}\n> -\t\tstd::vector {{field.mojom_name}}Fds;\n> +\t\tstd::vector {{field.mojom_name}}Fds;\n> \t\tstd::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) =\n> \t{%- else %}\n> \t\tstd::tie({{field.mojom_name}}, std::ignore) =\n> @@ -177,7 +177,7 @@\n> # \\a struct.\n> #}\n> {%- macro serializer(struct, namespace) %}\n> -\tstatic std::tuple, std::vector>\n> +\tstatic std::tuple, std::vector>\n> \tserialize(const {{struct|name_full}} &data,\n> {%- if struct|needs_control_serializer %}\n> \t\t ControlSerializer *cs)\n> @@ -187,7 +187,7 @@\n> \t{\n> \t\tstd::vector retData;\n> {%- if struct|has_fd %}\n> -\t\tstd::vector retFds;\n> +\t\tstd::vector retFds;\n> {%- endif %}\n> {%- for field in struct.fields %}\n> {{serializer_field(field, namespace, loop)}}\n> @@ -210,7 +210,7 @@\n> {%- macro deserializer_fd(struct, namespace) %}\n> \tstatic {{struct|name_full}}\n> \tdeserialize(std::vector &data,\n> -\t\t std::vector &fds,\n> +\t\t std::vector &fds,\n> {%- if struct|needs_control_serializer %}\n> \t\t ControlSerializer *cs)\n> {%- else %}\n> @@ -224,8 +224,8 @@\n> \tstatic {{struct|name_full}}\n> \tdeserialize(std::vector::const_iterator dataBegin,\n> \t\t std::vector::const_iterator dataEnd,\n> -\t\t std::vector::const_iterator fdsBegin,\n> -\t\t std::vector::const_iterator fdsEnd,\n> +\t\t std::vector::const_iterator fdsBegin,\n> +\t\t std::vector::const_iterator fdsEnd,\n> {%- if struct|needs_control_serializer %}\n> \t\t ControlSerializer *cs)\n> {%- else %}\n> @@ -234,7 +234,7 @@\n> \t{\n> \t\t{{struct|name_full}} ret;\n> \t\tstd::vector::const_iterator m = dataBegin;\n> -\t\tstd::vector::const_iterator n = fdsBegin;\n> +\t\tstd::vector::const_iterator n = fdsBegin;\n> \n> \t\tsize_t dataSize = std::distance(dataBegin, dataEnd);\n> \t\t[[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd);\n> @@ -255,7 +255,7 @@\n> {%- macro deserializer_fd_simple(struct, namespace) %}\n> \tstatic {{struct|name_full}}\n> \tdeserialize(std::vector &data,\n> -\t\t [[maybe_unused]] std::vector &fds,\n> +\t\t [[maybe_unused]] std::vector &fds,\n> \t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\treturn IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);\n> @@ -264,8 +264,8 @@\n> \tstatic {{struct|name_full}}\n> \tdeserialize(std::vector::const_iterator dataBegin,\n> \t\t std::vector::const_iterator dataEnd,\n> -\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> -\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\treturn IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9927AC322B\n\tfor ;\n\tWed, 21 Jul 2021 13:35:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1E41868545;\n\tWed, 21 Jul 2021 15:35:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73D3F60276\n\tfor ;\n\tWed, 21 Jul 2021 15:35:51 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CE390196A;\n\tWed, 21 Jul 2021 15:35:50 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HwWsV6qb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626874551;\n\tbh=z6FhmxliLlueQ/px1YBrqk7N0Wwl5MojiSIA1NOrxmI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HwWsV6qbO3unZdyQaVkldRkAEc/43Qvu+XpybSt1gvQyjb/WFUECvZ2TWJFkC7HIJ\n\tFAkf+pO9x3CdV3zRP/BHV8HueHyR48fwgocK14+2u1Aq2ZdgqvPHvM5JFmM6EWllwK\n\tNApfHobPockbkMoAOsk7bPAPqBjyhH98DHLuLlLE=","Date":"Wed, 21 Jul 2021 16:35:45 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210721102952.49279-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210721102952.49279-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] ipa: Use FileDescriptor instead of\n\tint in layers above IPC payload","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18276,"web_url":"https://patchwork.libcamera.org/comment/18276/","msgid":"<68fc2ea4-3fe5-8db9-b4aa-eb19fbdf3d93@ideasonboard.com>","date":"2021-07-23T08:08:08","subject":"Re: [libcamera-devel] [PATCH v2] ipa: Use FileDescriptor instead of\n\tint in layers above IPC payload","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nSorry for the delay since I was busy with other nautilus specific \nplatform issues.\n\nOn 7/21/21 3:59 PM, Paul Elder wrote:\n> Regarding (de)serialization in isolated IPA calls, we have four layers:\n> - struct\n> - byte vector + fd vector\n> - IPCMessage\n> - IPC payload\n>\n> The proxy handles the upper three layers (with help from the\n> IPADataSerializer), and passes an IPCMessage to the IPC mechanism\n> (implemented as an IPCPipe), which sends an IPC payload to its worker\n> counterpart.\n>\n> When a FileDescriptor is involved, previously it was only a\n> FileDescriptor in the first layer; in the lower three it was an int. To\n> reduce the risk of potential fd leaks in the future, keep the\n> FileDescriptor as-is throughout the upper three layers. Only the IPC\n> mechanism will deal with ints, if it so wishes, when it does the actual\n> IPC. IPCPipeUnixSocket does deal with ints for sending fds, so the\n> conversion between IPCMessage and IPCUnixSocket::Payload converts\n> between FileDescriptor and int.\n>\n> Signed-off-by: Paul Elder \n>\n> ---\n> Umang, could you please check if this fixes the issue?\n\n\nWith this patch on a branch, what I observe is the IPA is failing \nmapBuffers() after configure(), when run in the isolated mode :-(\n\nHence, it seems I cannot stream any frame in the in-built camera-app or \ncam. In either cases, following the log as per [1], the logs, I can \nreproduce the following:\n\n[1:24:51.084952361] [22042] DEBUG IPAProxyIPU3Worker \nipu3_ipa_proxy_worker.cpp:95 Done replying to init()\n[1:24:51.090070450] [22042] DEBUG IPAIPU3 ipu3.cpp:154 Best grid found \nis: (105 << 4) x (8 << 7)\n[1:24:51.090146582] [22042] DEBUG IPAProxyIPU3Worker \nipu3_ipa_proxy_worker.cpp:168 Done replying to configure()\n[1:24:51.095146306] [22042] ERROR Buffer framebuffer.cpp:381 Failed to \nmmap plane\n[1:24:51.095196216] [22042] ERROR Buffer framebuffer.cpp:381 Failed to \nmmap plane\n[1:24:51.095216802] [22042] ERROR Buffer framebuffer.cpp:381 Failed to \nmmap plane\n[1:24:51.095233834] [22042] ERROR Buffer framebuffer.cpp:381 Failed to \nmmap plane\n[1:24:51.095250375] [22042] ERROR Buffer framebuffer.cpp:381 Failed to \nmmap plane\n[1:24:51.095267364] [22042] ERROR Buffer framebuffer.cpp:381 Failed to \nmmap plane\n[1:24:51.095284167] [22042] ERROR Buffer framebuffer.cpp:381 Failed to \nmmap plane\n[1:24:51.095480975] [22042] DEBUG IPAProxyIPU3Worker \nipu3_ipa_proxy_worker.cpp:195 Done replying to mapBuffers()\n[1:24:51.095751564] [22042] DEBUG IPAProxyIPU3Worker \nipu3_ipa_proxy_worker.cpp:304 queueFrameAction done\n[1:24:51.095839505] [22042] DEBUG IPAProxyIPU3Worker \nipu3_ipa_proxy_worker.cpp:119 Done replying to start()\n\n[1] diff\n\n```\n\ndiff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex 71698d36..ad5b3732 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -20,6 +20,8 @@\n  #include \n  #include \n\n+#include \n+\n  #include \"libcamera/internal/framebuffer.h\"\n\n  #include \"ipu3_agc.h\"\n@@ -94,6 +96,7 @@ int IPAIPU3::init(const IPASettings &settings)\n                 return -ENODEV;\n         }\n\n+       logSetFile(\"/tmp/isolated.log\");\n         return 0;\n  }\n```\n\n> ---\n> .../libcamera/internal/ipa_data_serializer.h | 40 +++++-----\n> include/libcamera/internal/ipc_pipe.h | 10 ++-\n> src/libcamera/ipa_data_serializer.cpp | 78 +++++++++----------\n> src/libcamera/ipc_pipe.cpp | 12 ++-\n> .../ipa_data_serializer_test.cpp | 12 +--\n> .../module_ipa_proxy.cpp.tmpl | 2 +-\n> .../module_ipa_proxy.h.tmpl | 2 +-\n> .../libcamera_templates/proxy_functions.tmpl | 2 +-\n> .../libcamera_templates/serializer.tmpl | 22 +++---\n> 9 files changed, 90 insertions(+), 90 deletions(-)\n>\n> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h\n> index 519093bd..4353e07b 100644\n> --- a/include/libcamera/internal/ipa_data_serializer.h\n> +++ b/include/libcamera/internal/ipa_data_serializer.h\n> @@ -66,7 +66,7 @@ template\n> class IPADataSerializer\n> {\n> public:\n> -\tstatic std::tuple, std::vector>\n> +\tstatic std::tuple, std::vector>\n> \tserialize(const T &data, ControlSerializer *cs = nullptr);\n> \n> \tstatic T deserialize(const std::vector &data,\n> @@ -76,12 +76,12 @@ public:\n> \t\t\t ControlSerializer *cs = nullptr);\n> \n> \tstatic T deserialize(const std::vector &data,\n> -\t\t\t const std::vector &fds,\n> +\t\t\t const std::vector &fds,\n> \t\t\t ControlSerializer *cs = nullptr);\n> \tstatic T deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t std::vector::const_iterator fdsEnd,\n> +\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t std::vector::const_iterator fdsEnd,\n> \t\t\t ControlSerializer *cs = nullptr);\n> };\n> \n> @@ -104,11 +104,11 @@ template\n> class IPADataSerializer>\n> {\n> public:\n> -\tstatic std::tuple, std::vector>\n> +\tstatic std::tuple, std::vector>\n> \tserialize(const std::vector &data, ControlSerializer *cs = nullptr)\n> \t{\n> \t\tstd::vector dataVec;\n> -\t\tstd::vector fdsVec;\n> +\t\tstd::vector fdsVec;\n> \n> \t\t/* Serialize the length. */\n> \t\tuint32_t vecLen = data.size();\n> @@ -117,7 +117,7 @@ public:\n> \t\t/* Serialize the members. */\n> \t\tfor (auto const &it : data) {\n> \t\t\tstd::vector dvec;\n> -\t\t\tstd::vector fvec;\n> +\t\t\tstd::vector fvec;\n> \n> \t\t\tstd::tie(dvec, fvec) =\n> \t\t\t\tIPADataSerializer::serialize(it, cs);\n> @@ -141,11 +141,11 @@ public:\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \t\treturn deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);\n> \t}\n> \n> -\tstatic std::vector deserialize(std::vector &data, std::vector &fds,\n> +\tstatic std::vector deserialize(std::vector &data, std::vector &fds,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);\n> @@ -153,15 +153,15 @@ public:\n> \n> \tstatic std::vector deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\tuint32_t vecLen = readPOD(dataBegin, 0, dataEnd);\n> \t\tstd::vector ret(vecLen);\n> \n> \t\tstd::vector::const_iterator dataIter = dataBegin + 4;\n> -\t\tstd::vector::const_iterator fdIter = fdsBegin;\n> +\t\tstd::vector::const_iterator fdIter = fdsBegin;\n> \t\tfor (uint32_t i = 0; i < vecLen; i++) {\n> \t\t\tuint32_t sizeofData = readPOD(dataIter, 0, dataEnd);\n> \t\t\tuint32_t sizeofFds = readPOD(dataIter, 4, dataEnd);\n> @@ -201,11 +201,11 @@ template\n> class IPADataSerializer>\n> {\n> public:\n> -\tstatic std::tuple, std::vector>\n> +\tstatic std::tuple, std::vector>\n> \tserialize(const std::map &data, ControlSerializer *cs = nullptr)\n> \t{\n> \t\tstd::vector dataVec;\n> -\t\tstd::vector fdsVec;\n> +\t\tstd::vector fdsVec;\n> \n> \t\t/* Serialize the length. */\n> \t\tuint32_t mapLen = data.size();\n> @@ -214,7 +214,7 @@ public:\n> \t\t/* Serialize the members. */\n> \t\tfor (auto const &it : data) {\n> \t\t\tstd::vector dvec;\n> -\t\t\tstd::vector fvec;\n> +\t\t\tstd::vector fvec;\n> \n> \t\t\tstd::tie(dvec, fvec) =\n> \t\t\t\tIPADataSerializer::serialize(it.first, cs);\n> @@ -247,11 +247,11 @@ public:\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \t\treturn deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);\n> \t}\n> \n> -\tstatic std::map deserialize(std::vector &data, std::vector &fds,\n> +\tstatic std::map deserialize(std::vector &data, std::vector &fds,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);\n> @@ -259,8 +259,8 @@ public:\n> \n> \tstatic std::map deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\tstd::map ret;\n> @@ -268,7 +268,7 @@ public:\n> \t\tuint32_t mapLen = readPOD(dataBegin, 0, dataEnd);\n> \n> \t\tstd::vector::const_iterator dataIter = dataBegin + 4;\n> -\t\tstd::vector::const_iterator fdIter = fdsBegin;\n> +\t\tstd::vector::const_iterator fdIter = fdsBegin;\n> \t\tfor (uint32_t i = 0; i < mapLen; i++) {\n> \t\t\tuint32_t sizeofData = readPOD(dataIter, 0, dataEnd);\n> \t\t\tuint32_t sizeofFds = readPOD(dataIter, 4, dataEnd);\n> diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h\n> index e58de340..52cc8fa3 100644\n> --- a/include/libcamera/internal/ipc_pipe.h\n> +++ b/include/libcamera/internal/ipc_pipe.h\n> @@ -11,6 +11,8 @@\n> \n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/ipc_unixsocket.h\"\n> \n> namespace libcamera {\n> @@ -26,23 +28,23 @@ public:\n> \tIPCMessage();\n> \tIPCMessage(uint32_t cmd);\n> \tIPCMessage(const Header &header);\n> -\tIPCMessage(const IPCUnixSocket::Payload &payload);\n> +\tIPCMessage(IPCUnixSocket::Payload &payload);\n> \n> \tIPCUnixSocket::Payload payload() const;\n> \n> \tHeader &header() { return header_; }\n> \tstd::vector &data() { return data_; }\n> -\tstd::vector &fds() { return fds_; }\n> +\tstd::vector &fds() { return fds_; }\n> \n> \tconst Header &header() const { return header_; }\n> \tconst std::vector &data() const { return data_; }\n> -\tconst std::vector &fds() const { return fds_; }\n> +\tconst std::vector &fds() const { return fds_; }\n> \n> private:\n> \tHeader header_;\n> \n> \tstd::vector data_;\n> -\tstd::vector fds_;\n> +\tstd::vector fds_;\n> };\n> \n> class IPCPipe\n> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n> index fb941e6b..e991d751 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -8,6 +8,7 @@\n> #include \"libcamera/internal/ipa_data_serializer.h\"\n> \n> #include \n> +#include \n> \n> /**\n> * \\file ipa_data_serializer.h\n> @@ -141,7 +142,7 @@ namespace {\n> /**\n> * \\fn template IPADataSerializer::deserialize(\n> * \tconst std::vector &data,\n> - * \tconst std::vector &fds,\n> + * \tconst std::vector &fds,\n> * \tControlSerializer *cs = nullptr)\n> * \\brief Deserialize byte vector and fd vector into an object\n> * \\tparam T Type of object to deserialize to\n> @@ -162,8 +163,8 @@ namespace {\n> * \\fn template IPADataSerializer::deserialize(\n> * \tstd::vector::const_iterator dataBegin,\n> * \tstd::vector::const_iterator dataEnd,\n> - * \tstd::vector::const_iterator fdsBegin,\n> - * \tstd::vector::const_iterator fdsEnd,\n> + * \tstd::vector::const_iterator fdsBegin,\n> + * \tstd::vector::const_iterator fdsEnd,\n> * \tControlSerializer *cs = nullptr)\n> * \\brief Deserialize byte vector and fd vector into an object\n> * \\tparam T Type of object to deserialize to\n> @@ -187,7 +188,7 @@ namespace {\n> #define DEFINE_POD_SERIALIZER(type)\t\t\t\t\t\\\n> \t\t\t\t\t\t\t\t\t\\\n> template<>\t\t\t\t\t\t\t\t\\\n> -std::tuple, std::vector>\t\t\t\\\n> +std::tuple, std::vector>\t\t\t\\\n> IPADataSerializer::serialize(const type &data,\t\t\t\\\n> \t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> {\t\t\t\t\t\t\t\t\t\\\n> @@ -215,7 +216,7 @@ type IPADataSerializer::deserialize(const std::vector &data, \\\n> \t\t\t\t\t\t\t\t\t\\\n> template<>\t\t\t\t\t\t\t\t\\\n> type IPADataSerializer::deserialize(const std::vector &data, \\\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n> +\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n> \t\t\t\t\t ControlSerializer *cs)\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> \treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n> @@ -224,8 +225,8 @@ type IPADataSerializer::deserialize(const std::vector &data, \\\n> template<>\t\t\t\t\t\t\t\t\\\n> type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> \t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n> \t\t\t\t\t ControlSerializer *cs)\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> \treturn deserialize(dataBegin, dataEnd, cs);\t\t\t\\\n> @@ -249,7 +250,7 @@ DEFINE_POD_SERIALIZER(double)\n> * function parameter serdes).\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const std::string &data,\n> \t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> @@ -276,7 +277,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator\n> template<>\n> std::string\n> IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> +\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> \t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \treturn { data.cbegin(), data.cend() };\n> @@ -286,8 +287,8 @@ template<>\n> std::string\n> IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \treturn { dataBegin, dataEnd };\n> @@ -305,7 +306,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator\n> * be used. The serialized ControlInfoMap will have zero length.\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const ControlList &data, ControlSerializer *cs)\n> {\n> \tif (!cs)\n> @@ -405,7 +406,7 @@ IPADataSerializer::deserialize(const std::vector &data,\n> template<>\n> ControlList\n> IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> +\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(data.cbegin(), data.end(), cs);\n> @@ -415,8 +416,8 @@ template<>\n> ControlList\n> IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(dataBegin, dataEnd, cs);\n> @@ -429,7 +430,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator\n> * X bytes - Serialized ControlInfoMap (using ControlSerializer)\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const ControlInfoMap &map,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> @@ -491,7 +492,7 @@ IPADataSerializer::deserialize(const std::vector &data,\n> template<>\n> ControlInfoMap\n> IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> +\t\t\t\t\t [[maybe_unused]] const std::vector &fds,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(data.cbegin(), data.end(), cs);\n> @@ -501,8 +502,8 @@ template<>\n> ControlInfoMap\n> IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(dataBegin, dataEnd, cs);\n> @@ -522,37 +523,28 @@ IPADataSerializer::deserialize(std::vector::const_itera\n> * 32-bit alignment of all serialized data\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const FileDescriptor &data,\n> \t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> -\tstd::vector dataVec = { data.isValid() };\n> -\tstd::vector fdVec;\n> -\tif (data.isValid())\n> -\t\tfdVec.push_back(data.fd());\n> -\n> -\treturn { dataVec, fdVec };\n> +\treturn { {}, { data } };\n> }\n> \n> template<>\n> -FileDescriptor IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> -\t\t\t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t\t\t std::vector::const_iterator fdsEnd,\n> +FileDescriptor IPADataSerializer::deserialize([[maybe_unused]] std::vector::const_iterator dataBegin,\n> +\t\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator dataEnd,\n> +\t\t\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t\t\t std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> -\tASSERT(std::distance(dataBegin, dataEnd) >= 1);\n> -\n> -\tbool valid = !!(*dataBegin);\n> -\n> -\tASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));\n> +\tASSERT(std::distance(fdsBegin, fdsEnd) == 1);\n> \n> -\treturn valid ? FileDescriptor(*fdsBegin) : FileDescriptor();\n> +\treturn *fdsBegin;\n> }\n> \n> template<>\n> FileDescriptor IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t\t\t const std::vector &fds,\n> +\t\t\t\t\t\t\t const std::vector &fds,\n> \t\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end());\n> @@ -565,15 +557,15 @@ FileDescriptor IPADataSerializer::deserialize(const std::vector<\n> * 4 bytes - uint32_t Length\n> */\n> template<>\n> -std::tuple, std::vector>\n> +std::tuple, std::vector>\n> IPADataSerializer::serialize(const FrameBuffer::Plane &data,\n> \t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \tstd::vector dataVec;\n> -\tstd::vector fdsVec;\n> +\tstd::vector fdsVec;\n> \n> \tstd::vector fdBuf;\n> -\tstd::vector fdFds;\n> +\tstd::vector fdFds;\n> \tstd::tie(fdBuf, fdFds) =\n> \t\tIPADataSerializer::serialize(data.fd);\n> \tdataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end());\n> @@ -588,8 +580,8 @@ template<>\n> FrameBuffer::Plane\n> IPADataSerializer::deserialize(std::vector::const_iterator dataBegin,\n> \t\t\t\t\t\t std::vector::const_iterator dataEnd,\n> -\t\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> -\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t\t\t\t std::vector::const_iterator fdsBegin,\n> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n> {\n> \tFrameBuffer::Plane ret;\n> @@ -604,7 +596,7 @@ IPADataSerializer::deserialize(std::vector::const_i\n> template<>\n> FrameBuffer::Plane\n> IPADataSerializer::deserialize(const std::vector &data,\n> -\t\t\t\t\t\t const std::vector &fds,\n> +\t\t\t\t\t\t const std::vector &fds,\n> \t\t\t\t\t\t ControlSerializer *cs)\n> {\n> \treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);\n> diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp\n> index 28e20e03..84136a82 100644\n> --- a/src/libcamera/ipc_pipe.cpp\n> +++ b/src/libcamera/ipc_pipe.cpp\n> @@ -77,13 +77,17 @@ IPCMessage::IPCMessage(const Header &header)\n> *\n> * This essentially converts an IPCUnixSocket payload into an IPCMessage.\n> * The header is extracted from the payload into the IPCMessage's header field.\n> + *\n> + * If the IPCUnixSocket payload had any valid file descriptors, then they will\n> + * all be invalidated.\n> */\n> -IPCMessage::IPCMessage(const IPCUnixSocket::Payload &payload)\n> +IPCMessage::IPCMessage(IPCUnixSocket::Payload &payload)\n> {\n> \tmemcpy(&header_, payload.data.data(), sizeof(header_));\n> \tdata_ = std::vector(payload.data.begin() + sizeof(header_),\n> \t\t\t\t payload.data.end());\n> -\tfds_ = payload.fds;\n> +\tfor (int32_t &fd : payload.fds)\n> +\t\tfds_.push_back(FileDescriptor(std::move(fd)));\n> }\n> \n> /**\n> @@ -104,7 +108,9 @@ IPCUnixSocket::Payload IPCMessage::payload() const\n> \n> \t/* \\todo Make this work without copy */\n> \tmemcpy(payload.data.data() + sizeof(Header), data_.data(), data_.size());\n> -\tpayload.fds = fds_;\n> +\n> +\tfor (const FileDescriptor &fd : fds_)\n> +\t\tpayload.fds.push_back(fd.fd());\n> \n> \treturn payload;\n> }\n> diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp\n> index 880bcd02..1e8f7e21 100644\n> --- a/test/serialization/ipa_data_serializer_test.cpp\n> +++ b/test/serialization/ipa_data_serializer_test.cpp\n> @@ -53,7 +53,7 @@ template\n> int testPodSerdes(T in)\n> {\n> \tstd::vector buf;\n> -\tstd::vector fds;\n> +\tstd::vector fds;\n> \n> \tstd::tie(buf, fds) = IPADataSerializer::serialize(in);\n> \tT out = IPADataSerializer::deserialize(buf, fds);\n> @@ -72,7 +72,7 @@ int testVectorSerdes(const std::vector &in,\n> \t\t ControlSerializer *cs = nullptr)\n> {\n> \tstd::vector buf;\n> -\tstd::vector fds;\n> +\tstd::vector fds;\n> \n> \tstd::tie(buf, fds) = IPADataSerializer>::serialize(in, cs);\n> \tstd::vector out = IPADataSerializer>::deserialize(buf, fds, cs);\n> @@ -92,7 +92,7 @@ int testMapSerdes(const std::map &in,\n> \t\t ControlSerializer *cs = nullptr)\n> {\n> \tstd::vector buf;\n> -\tstd::vector fds;\n> +\tstd::vector fds;\n> \n> \tstd::tie(buf, fds) = IPADataSerializer>::serialize(in, cs);\n> \tstd::map out = IPADataSerializer>::deserialize(buf, fds, cs);\n> @@ -219,7 +219,7 @@ private:\n> \t\t};\n> \n> \t\tstd::vector buf;\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \n> \t\tif (testVectorSerdes(vecUint8) != TestPass)\n> \t\t\treturn TestFail;\n> @@ -291,7 +291,7 @@ private:\n> \t\t\t{ { \"a\", { 1, 2, 3 } }, { \"b\", { 4, 5, 6 } }, { \"c\", { 7, 8, 9 } } };\n> \n> \t\tstd::vector buf;\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \n> \t\tif (testMapSerdes(mapUintStr) != TestPass)\n> \t\t\treturn TestFail;\n> @@ -359,7 +359,7 @@ private:\n> \t\tstd::string strEmpty = \"\";\n> \n> \t\tstd::vector buf;\n> -\t\tstd::vector fds;\n> +\t\tstd::vector fds;\n> \n> \t\tif (testPodSerdes(u32min) != TestPass)\n> \t\t\treturn TestFail;\n> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> index a4e008c7..aab1fffb 100644\n> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> @@ -236,7 +236,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)\n> void {{proxy_name}}::{{method.mojom_name}}IPC(\n> \tstd::vector::const_iterator data,\n> \tsize_t dataSize,\n> -\t[[maybe_unused]] const std::vector &fds)\n> +\t[[maybe_unused]] const std::vector &fds)\n> {\n> {%- for param in method.parameters %}\n> \t{{param|name}} {{param.mojom_name}};\n> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> index ae168548..09e3af7d 100644\n> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> @@ -65,7 +65,7 @@ private:\n> \tvoid {{method.mojom_name}}IPC(\n> \t\tstd::vector::const_iterator data,\n> \t\tsize_t dataSize,\n> -\t\tconst std::vector &fds);\n> +\t\tconst std::vector &fds);\n> {% endfor %}\n> \n> \t/* Helper class to invoke async functions in another thread. */\n> diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> index ea9cc86b..ebcd2aaa 100644\n> --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> @@ -54,7 +54,7 @@\n> {%- for param in params %}\n> \tstd::vector {{param.mojom_name}}Buf;\n> {%- if param|has_fd %}\n> -\tstd::vector {{param.mojom_name}}Fds;\n> +\tstd::vector {{param.mojom_name}}Fds;\n> \tstd::tie({{param.mojom_name}}Buf, {{param.mojom_name}}Fds) =\n> {%- else %}\n> \tstd::tie({{param.mojom_name}}Buf, std::ignore) =\n> diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> index d8d55807..cdd31df8 100644\n> --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> @@ -40,7 +40,7 @@\n> \t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n> {%- elif field|is_fd %}\n> \t\tstd::vector {{field.mojom_name}};\n> -\t\tstd::vector {{field.mojom_name}}Fds;\n> +\t\tstd::vector {{field.mojom_name}}Fds;\n> \t\tstd::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) =\n> \t\t\tIPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});\n> \t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n> @@ -58,7 +58,7 @@\n> {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}\n> \t\tstd::vector {{field.mojom_name}};\n> \t{%- if field|has_fd %}\n> -\t\tstd::vector {{field.mojom_name}}Fds;\n> +\t\tstd::vector {{field.mojom_name}}Fds;\n> \t\tstd::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) =\n> \t{%- else %}\n> \t\tstd::tie({{field.mojom_name}}, std::ignore) =\n> @@ -177,7 +177,7 @@\n> # \\a struct.\n> #}\n> {%- macro serializer(struct, namespace) %}\n> -\tstatic std::tuple, std::vector>\n> +\tstatic std::tuple, std::vector>\n> \tserialize(const {{struct|name_full}} &data,\n> {%- if struct|needs_control_serializer %}\n> \t\t ControlSerializer *cs)\n> @@ -187,7 +187,7 @@\n> \t{\n> \t\tstd::vector retData;\n> {%- if struct|has_fd %}\n> -\t\tstd::vector retFds;\n> +\t\tstd::vector retFds;\n> {%- endif %}\n> {%- for field in struct.fields %}\n> {{serializer_field(field, namespace, loop)}}\n> @@ -210,7 +210,7 @@\n> {%- macro deserializer_fd(struct, namespace) %}\n> \tstatic {{struct|name_full}}\n> \tdeserialize(std::vector &data,\n> -\t\t std::vector &fds,\n> +\t\t std::vector &fds,\n> {%- if struct|needs_control_serializer %}\n> \t\t ControlSerializer *cs)\n> {%- else %}\n> @@ -224,8 +224,8 @@\n> \tstatic {{struct|name_full}}\n> \tdeserialize(std::vector::const_iterator dataBegin,\n> \t\t std::vector::const_iterator dataEnd,\n> -\t\t std::vector::const_iterator fdsBegin,\n> -\t\t std::vector::const_iterator fdsEnd,\n> +\t\t std::vector::const_iterator fdsBegin,\n> +\t\t std::vector::const_iterator fdsEnd,\n> {%- if struct|needs_control_serializer %}\n> \t\t ControlSerializer *cs)\n> {%- else %}\n> @@ -234,7 +234,7 @@\n> \t{\n> \t\t{{struct|name_full}} ret;\n> \t\tstd::vector::const_iterator m = dataBegin;\n> -\t\tstd::vector::const_iterator n = fdsBegin;\n> +\t\tstd::vector::const_iterator n = fdsBegin;\n> \n> \t\tsize_t dataSize = std::distance(dataBegin, dataEnd);\n> \t\t[[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd);\n> @@ -255,7 +255,7 @@\n> {%- macro deserializer_fd_simple(struct, namespace) %}\n> \tstatic {{struct|name_full}}\n> \tdeserialize(std::vector &data,\n> -\t\t [[maybe_unused]] std::vector &fds,\n> +\t\t [[maybe_unused]] std::vector &fds,\n> \t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\treturn IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);\n> @@ -264,8 +264,8 @@\n> \tstatic {{struct|name_full}}\n> \tdeserialize(std::vector::const_iterator dataBegin,\n> \t\t std::vector::const_iterator dataEnd,\n> -\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> -\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> \t\t ControlSerializer *cs = nullptr)\n> \t{\n> \t\treturn IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9A1BBC0109\n\tfor ;\n\tFri, 23 Jul 2021 08:08:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 01CA1687A6;\n\tFri, 23 Jul 2021 10:08:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C9CB6853F\n\tfor ;\n\tFri, 23 Jul 2021 10:08:14 +0200 (CEST)","from [192.168.0.107] (unknown [103.251.226.103])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 53E683F2;\n\tFri, 23 Jul 2021 10:08:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"im/66yDX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627027694;\n\tbh=GNaJWFiKOI4G89DePxOe+hTS0sTJ46IvYMZi0V4ElLU=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=im/66yDXCXEDrsVHQ6wBrzi6jD5fVlVF9FKmpTT3TdAbnHXHvfe94sY+iTHFiwTs1\n\toY2ev9+/My1/z6LZ9etIHjL1UnRILfc6xKmRHoDRIC3viOIgHhEEG6Dy38jqujc1JZ\n\t2hc22/tmk8VoY9anUejC/v6jSSht/etyg0t0eZvk=","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210721102952.49279-1-paul.elder@ideasonboard.com>","From":"Umang Jain ","Message-ID":"<68fc2ea4-3fe5-8db9-b4aa-eb19fbdf3d93@ideasonboard.com>","Date":"Fri, 23 Jul 2021 13:38:08 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210721102952.49279-1-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2] ipa: Use FileDescriptor instead of\n\tint in layers above IPC payload","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]