[{"id":18866,"web_url":"https://patchwork.libcamera.org/comment/18866/","msgid":"<3d76490e-a1df-0667-eb7f-7f1f19ad1d1e@ideasonboard.com>","date":"2021-08-17T12:34:15","subject":"Re: [libcamera-devel] [PATCH v3] 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\nOn 8/17/21 10:16 AM, 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> Additionally, change the data portion of the serialized form of\n> FileDescriptor to a 32-bit unsigned integer, for alightnment purposes\n> and in preparation for conversion to an index into the fd array.\n>\n> Signed-off-by: Paul Elder \n\nThe patch passes the Fd leak test v1 so:\n\nTested-by: Umang Jain \n\n>\n> ---\n> Changes in v3:\n> - encode isValid as a uint32\n> ---\n> .../libcamera/internal/ipa_data_serializer.h | 40 ++++-----\n> include/libcamera/internal/ipc_pipe.h | 10 ++-\n> src/libcamera/ipa_data_serializer.cpp | 86 +++++++++++--------\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 | 26 +++---\n> 9 files changed, 105 insertions(+), 87 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..91471f52 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -7,6 +7,8 @@\n> \n> #include \"libcamera/internal/ipa_data_serializer.h\"\n> \n> +#include \n> +\n> #include \n> \n> /**\n> @@ -141,7 +143,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 +164,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 +189,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\\\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 +217,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 +226,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 +251,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 +278,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 +288,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 +307,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 +407,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 +417,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 +431,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 +493,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 +503,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 +524,45 @@ 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> +\tstd::vector dataVec;\n> +\tstd::vector fdVec;\n> +\n> +\t/*\n> +\t * Store as uint32_t to prepare for conversion from validity flag\n> +\t * to index, and for alignment\n> +\t */\n> +\tappendPOD(dataVec, data.isValid());\n> +\n> \tif (data.isValid())\n> -\t\tfdVec.push_back(data.fd());\n> +\t\tfdVec.push_back(data);\n> +\n> \n> \treturn { dataVec, fdVec };\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> +\tASSERT(std::distance(dataBegin, dataEnd) >= 4);\n> \n> -\tbool valid = !!(*dataBegin);\n> +\tuint32_t valid = readPOD(dataBegin, 0, dataEnd);\n> \n> \tASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));\n> \n> -\treturn valid ? FileDescriptor(*fdsBegin) : FileDescriptor();\n> +\treturn valid ? *fdsBegin : FileDescriptor();\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 +575,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,13 +598,13 @@ 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> \n> -\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 1,\n> +\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n> \t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n> \tret.length = readPOD(dataBegin, 1, dataEnd);\n> \n> @@ -604,7 +614,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 bf7e34e2..eca19a66 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 c222f5f2..1979e68f 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..b8ef8e7b 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> @@ -104,9 +104,9 @@\n> \t\tdataSize -= {{field_size}};\n> \t{%- endif %}\n> {% elif field|is_fd %}\n> -\t{%- set field_size = 1 %}\n> +\t{%- set field_size = 4 %}\n> \t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> -\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs);\n> +\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs);\n> \t{%- if not loop.last %}\n> \t\tm += {{field_size}};\n> \t\tdataSize -= {{field_size}};\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 87C70BD87D\n\tfor ;\n\tTue, 17 Aug 2021 12:34:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED9CC688A5;\n\tTue, 17 Aug 2021 14:34:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B17486025C\n\tfor ;\n\tTue, 17 Aug 2021 14:34:20 +0200 (CEST)","from [192.168.1.104] (unknown [103.238.109.15])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 94D1A2C5;\n\tTue, 17 Aug 2021 14:34:19 +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=\"NUp8SlLV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629203660;\n\tbh=ID5nZlewEavzzcZlLT2KCMSUYD5MG5ohB4zxyx/V94M=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=NUp8SlLVGqxDg8tjMj5jy6iSynitOOrlZ3x1ngIIMck/eE81xT+51gJ8rNj6ha1Bw\n\tnsatMOzQYqWsLP/xQvB/rzPv1P8nC0xGrVkxnyOWuldUEzUjxeY7efzN4p4zdzb3yf\n\t1BR14vp562j0E41c00SqGoziQIXMR/fXcsyBJ7l4=","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210817044633.2061596-1-paul.elder@ideasonboard.com>","From":"Umang Jain ","Message-ID":"<3d76490e-a1df-0667-eb7f-7f1f19ad1d1e@ideasonboard.com>","Date":"Tue, 17 Aug 2021 18:04:15 +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":"<20210817044633.2061596-1-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3] 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\" "}},{"id":18868,"web_url":"https://patchwork.libcamera.org/comment/18868/","msgid":"","date":"2021-08-17T12:49:17","subject":"Re: [libcamera-devel] [PATCH v3] 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 Tue, Aug 17, 2021 at 01:46:33PM +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> Additionally, change the data portion of the serialized form of\n> FileDescriptor to a 32-bit unsigned integer, for alightnment purposes\n> and in preparation for conversion to an index into the fd array.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v3:\n> - encode isValid as a uint32\n> ---\n> .../libcamera/internal/ipa_data_serializer.h | 40 ++++-----\n> include/libcamera/internal/ipc_pipe.h | 10 ++-\n> src/libcamera/ipa_data_serializer.cpp | 86 +++++++++++--------\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 | 26 +++---\n> 9 files changed, 105 insertions(+), 87 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..91471f52 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -7,6 +7,8 @@\n> \n> #include \"libcamera/internal/ipa_data_serializer.h\"\n> \n> +#include \n> +\n> #include \n> \n> /**\n> @@ -141,7 +143,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 +164,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 +189,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\\\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 +217,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 +226,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 +251,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 +278,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 +288,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 +307,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 +407,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 +417,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 +431,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 +493,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 +503,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 +524,45 @@ 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> +\tstd::vector dataVec;\n> +\tstd::vector fdVec;\n> +\n> +\t/*\n> +\t * Store as uint32_t to prepare for conversion from validity flag\n> +\t * to index, and for alignment\n\ns/alignment/alignment./\n\nReviewed-by: Laurent Pinchart \n\n> +\t */\n> +\tappendPOD(dataVec, data.isValid());\n> +\n> \tif (data.isValid())\n> -\t\tfdVec.push_back(data.fd());\n> +\t\tfdVec.push_back(data);\n> +\n> \n> \treturn { dataVec, fdVec };\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> +\tASSERT(std::distance(dataBegin, dataEnd) >= 4);\n> \n> -\tbool valid = !!(*dataBegin);\n> +\tuint32_t valid = readPOD(dataBegin, 0, dataEnd);\n> \n> \tASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));\n> \n> -\treturn valid ? FileDescriptor(*fdsBegin) : FileDescriptor();\n> +\treturn valid ? *fdsBegin : FileDescriptor();\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 +575,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,13 +598,13 @@ 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> \n> -\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 1,\n> +\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n> \t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n> \tret.length = readPOD(dataBegin, 1, dataEnd);\n> \n> @@ -604,7 +614,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 bf7e34e2..eca19a66 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 c222f5f2..1979e68f 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..b8ef8e7b 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> @@ -104,9 +104,9 @@\n> \t\tdataSize -= {{field_size}};\n> \t{%- endif %}\n> {% elif field|is_fd %}\n> -\t{%- set field_size = 1 %}\n> +\t{%- set field_size = 4 %}\n> \t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> -\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs);\n> +\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs);\n> \t{%- if not loop.last %}\n> \t\tm += {{field_size}};\n> \t\tdataSize -= {{field_size}};\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 D3DA2BD87D\n\tfor ;\n\tTue, 17 Aug 2021 12:49:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 48962688A5;\n\tTue, 17 Aug 2021 14:49:25 +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 DD1706025C\n\tfor ;\n\tTue, 17 Aug 2021 14:49:23 +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 5283B2C5;\n\tTue, 17 Aug 2021 14:49:23 +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=\"OrXQD5M5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629204563;\n\tbh=uuy+KVG2KeHytLdyyPT6Gk+ecX+CQ7WYPM7Dx3lohTE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OrXQD5M5SKKqMYpK8U/77WPdr3VXF6hkZqtf/VUBmKk+hYi2vefgYnerNKwJQtv38\n\t2UpP9cVBV5/JMwhozBfq6eZRG4GZWB9DIXQfExZOUo4NW95n4Hbx68aQpHF4LgOrqH\n\tJo98V8M6zOs9nmus/5QpMQDVrg+trDtAWsaxvjBQ=","Date":"Tue, 17 Aug 2021 15:49:17 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210817044633.2061596-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210817044633.2061596-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3] 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":18880,"web_url":"https://patchwork.libcamera.org/comment/18880/","msgid":"","date":"2021-08-17T23:25:25","subject":"Re: [libcamera-devel] [PATCH v3] 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\nAnother comment.\n\nOn Tue, Aug 17, 2021 at 03:49:18PM +0300, Laurent Pinchart wrote:\n> On Tue, Aug 17, 2021 at 01:46:33PM +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> > Additionally, change the data portion of the serialized form of\n> > FileDescriptor to a 32-bit unsigned integer, for alightnment purposes\n> > and in preparation for conversion to an index into the fd array.\n> > \n> > Signed-off-by: Paul Elder \n> > \n> > ---\n> > Changes in v3:\n> > - encode isValid as a uint32\n> > ---\n> > .../libcamera/internal/ipa_data_serializer.h | 40 ++++-----\n> > include/libcamera/internal/ipc_pipe.h | 10 ++-\n> > src/libcamera/ipa_data_serializer.cpp | 86 +++++++++++--------\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 | 26 +++---\n> > 9 files changed, 105 insertions(+), 87 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..91471f52 100644\n> > --- a/src/libcamera/ipa_data_serializer.cpp\n> > +++ b/src/libcamera/ipa_data_serializer.cpp\n> > @@ -7,6 +7,8 @@\n> > \n> > #include \"libcamera/internal/ipa_data_serializer.h\"\n> > \n> > +#include \n> > +\n> > #include \n> > \n> > /**\n> > @@ -141,7 +143,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 +164,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 +189,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\\\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 +217,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 +226,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 +251,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 +278,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 +288,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 +307,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 +407,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 +417,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 +431,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 +493,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 +503,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 +524,45 @@ 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> > +\tstd::vector dataVec;\n> > +\tstd::vector fdVec;\n> > +\n> > +\t/*\n> > +\t * Store as uint32_t to prepare for conversion from validity flag\n> > +\t * to index, and for alignment\n> \n> s/alignment/alignment./\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > +\t */\n> > +\tappendPOD(dataVec, data.isValid());\n> > +\n> > \tif (data.isValid())\n> > -\t\tfdVec.push_back(data.fd());\n> > +\t\tfdVec.push_back(data);\n> > +\n> > \n> > \treturn { dataVec, fdVec };\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> > +\tASSERT(std::distance(dataBegin, dataEnd) >= 4);\n> > \n> > -\tbool valid = !!(*dataBegin);\n> > +\tuint32_t valid = readPOD(dataBegin, 0, dataEnd);\n> > \n> > \tASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));\n> > \n> > -\treturn valid ? FileDescriptor(*fdsBegin) : FileDescriptor();\n> > +\treturn valid ? *fdsBegin : FileDescriptor();\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 +575,15 @@ FileDescriptor IPADataSerializer::deserialize(const std::vector<\n\nThe comment here mentions 1 byte for the file descriptor, while it's now\n4 bytes. Maybe the comment could be dropped altogether, up to you.\n\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,13 +598,13 @@ 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> > \n> > -\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 1,\n> > +\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n> > \t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n> > \tret.length = readPOD(dataBegin, 1, dataEnd);\n> > \n> > @@ -604,7 +614,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 bf7e34e2..eca19a66 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 c222f5f2..1979e68f 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..b8ef8e7b 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> > @@ -104,9 +104,9 @@\n> > \t\tdataSize -= {{field_size}};\n> > \t{%- endif %}\n> > {% elif field|is_fd %}\n> > -\t{%- set field_size = 1 %}\n> > +\t{%- set field_size = 4 %}\n> > \t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> > -\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs);\n> > +\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs);\n> > \t{%- if not loop.last %}\n> > \t\tm += {{field_size}};\n> > \t\tdataSize -= {{field_size}};\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 0A74BBD87C\n\tfor ;\n\tTue, 17 Aug 2021 23:25:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 747E168894;\n\tWed, 18 Aug 2021 01:25:34 +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 2F4566025D\n\tfor ;\n\tWed, 18 Aug 2021 01:25:33 +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 8A5A7499;\n\tWed, 18 Aug 2021 01:25:32 +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=\"Qg0D4aFK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629242732;\n\tbh=mHCm72QugOe+kL6h8t9K7yqh4GhKsn3qMlVSgqPZUoQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Qg0D4aFKyxCaIHo8ZI5jPya12l2QZoAdPubO4Ar1/LDxA1UPkTpCcaNfErI9byaGj\n\t/hlZIjm5KHthiTqu4pPs76Xe+mE2hWD482I4EnsTEDHMSluvUEOprPq7Z85Aoyz3te\n\t/oCn/sfiMSX5RQU0i8qMt6JQjZV0jrh2ZHj8GYsc=","Date":"Wed, 18 Aug 2021 02:25:25 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210817044633.2061596-1-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3] 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":18938,"web_url":"https://patchwork.libcamera.org/comment/18938/","msgid":"","date":"2021-08-19T06:05:13","subject":"Re: [libcamera-devel] [PATCH v3] 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\nOn 8/17/21 10:16 AM, 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> Additionally, change the data portion of the serialized form of\n> FileDescriptor to a 32-bit unsigned integer, for alightnment purposes\n> and in preparation for conversion to an index into the fd array.\n>\n> Signed-off-by: Paul Elder \n\n\nReviewed-by: Umang Jain \n\nDo you any preferences of merging this? I plan to merge the fd-leak test \nsometime today (so it'll fail / reproducing the issue), so this fix can \nbe applied over it. I don't want a failing test lingering in master for \ntoo long, so I think merging this patch after the test is in, should be \nokay? Any thoughts? I can handle all the merging if you want that, as \nwell :-)\n\n\n>\n> ---\n> Changes in v3:\n> - encode isValid as a uint32\n> ---\n> .../libcamera/internal/ipa_data_serializer.h | 40 ++++-----\n> include/libcamera/internal/ipc_pipe.h | 10 ++-\n> src/libcamera/ipa_data_serializer.cpp | 86 +++++++++++--------\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 | 26 +++---\n> 9 files changed, 105 insertions(+), 87 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..91471f52 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -7,6 +7,8 @@\n> \n> #include \"libcamera/internal/ipa_data_serializer.h\"\n> \n> +#include \n> +\n> #include \n> \n> /**\n> @@ -141,7 +143,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 +164,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 +189,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\\\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 +217,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 +226,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 +251,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 +278,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 +288,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 +307,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 +407,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 +417,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 +431,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 +493,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 +503,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 +524,45 @@ 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> +\tstd::vector dataVec;\n> +\tstd::vector fdVec;\n> +\n> +\t/*\n> +\t * Store as uint32_t to prepare for conversion from validity flag\n> +\t * to index, and for alignment\n> +\t */\n> +\tappendPOD(dataVec, data.isValid());\n> +\n> \tif (data.isValid())\n> -\t\tfdVec.push_back(data.fd());\n> +\t\tfdVec.push_back(data);\n> +\n> \n> \treturn { dataVec, fdVec };\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> +\tASSERT(std::distance(dataBegin, dataEnd) >= 4);\n> \n> -\tbool valid = !!(*dataBegin);\n> +\tuint32_t valid = readPOD(dataBegin, 0, dataEnd);\n> \n> \tASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));\n> \n> -\treturn valid ? FileDescriptor(*fdsBegin) : FileDescriptor();\n> +\treturn valid ? *fdsBegin : FileDescriptor();\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 +575,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,13 +598,13 @@ 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> \n> -\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 1,\n> +\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n> \t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n> \tret.length = readPOD(dataBegin, 1, dataEnd);\n> \n> @@ -604,7 +614,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 bf7e34e2..eca19a66 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 c222f5f2..1979e68f 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..b8ef8e7b 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> @@ -104,9 +104,9 @@\n> \t\tdataSize -= {{field_size}};\n> \t{%- endif %}\n> {% elif field|is_fd %}\n> -\t{%- set field_size = 1 %}\n> +\t{%- set field_size = 4 %}\n> \t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> -\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs);\n> +\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs);\n> \t{%- if not loop.last %}\n> \t\tm += {{field_size}};\n> \t\tdataSize -= {{field_size}};\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 AC8A5BD87C\n\tfor ;\n\tThu, 19 Aug 2021 06:05:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC7AA68893;\n\tThu, 19 Aug 2021 08:05:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06265605AB\n\tfor ;\n\tThu, 19 Aug 2021 08:05:19 +0200 (CEST)","from [192.168.1.104] (unknown [103.238.109.15])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DD9932A8;\n\tThu, 19 Aug 2021 08:05:17 +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=\"nBWqJ81z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629353118;\n\tbh=zrxaHGqPO+9jlPgqbOpFl5oCBAeKAjh8aMTqeCCB/HE=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=nBWqJ81zcZ1KSoLHo7NVCwHGdJYn06R4htjx+ziVf1vCX3bTZa494E5HyC70HifK1\n\tkS2KW5NNLAgWiFPPrr3RAmVPhOSJGNWgNpo/5q6Vf6rwSPRXw7caiuAquDnW3nqWqF\n\tqXpTGLo9dPGdRXZxJNldD/8dFAG5wHxi03X7KVHk=","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210817044633.2061596-1-paul.elder@ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Thu, 19 Aug 2021 11:35:13 +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":"<20210817044633.2061596-1-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3] 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\" "}},{"id":18940,"web_url":"https://patchwork.libcamera.org/comment/18940/","msgid":"<20210819071340.GM1733965@pyrite.rasen.tech>","date":"2021-08-19T07:13:40","subject":"Re: [libcamera-devel] [PATCH v3] ipa: Use FileDescriptor instead of\n\tint in layers above IPC payload","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Thu, Aug 19, 2021 at 11:35:13AM +0530, Umang Jain wrote:\n> Hi Paul,\n> \n> On 8/17/21 10:16 AM, 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> > Additionally, change the data portion of the serialized form of\n> > FileDescriptor to a 32-bit unsigned integer, for alightnment purposes\n> > and in preparation for conversion to an index into the fd array.\n> > \n> > Signed-off-by: Paul Elder \n> \n> \n> Reviewed-by: Umang Jain \n> \n> Do you any preferences of merging this? I plan to merge the fd-leak test\n\nI was just waiting for one more review before pushing it, thanks :)\n\n> sometime today (so it'll fail / reproducing the issue), so this fix can be\n> applied over it. I don't want a failing test lingering in master for too\n> long, so I think merging this patch after the test is in, should be okay?\n> Any thoughts? I can handle all the merging if you want that, as well :-)\n\nHiro's patch actually reminded me that I need to fix the deserializer\noffset for FrameBuffer::plane, so I'll send a new version just so that\nit can be tested first. I'll push it after that's approved.\n\n\nThanks,\n\nPaul\n\n> \n> \n> > \n> > ---\n> > Changes in v3:\n> > - encode isValid as a uint32\n> > ---\n> > .../libcamera/internal/ipa_data_serializer.h | 40 ++++-----\n> > include/libcamera/internal/ipc_pipe.h | 10 ++-\n> > src/libcamera/ipa_data_serializer.cpp | 86 +++++++++++--------\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 | 26 +++---\n> > 9 files changed, 105 insertions(+), 87 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> > \tstatic T deserialize(const std::vector &data,\n> > @@ -76,12 +76,12 @@ public:\n> > \t\t\t ControlSerializer *cs = nullptr);\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> > @@ -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> > \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> > \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> > -\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> > \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> > \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> > \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> > \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> > -\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> > \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> > \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> > #include \n> > +#include \n> > +\n> > #include \"libcamera/internal/ipc_unixsocket.h\"\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> > \tIPCUnixSocket::Payload payload() const;\n> > \tHeader &header() { return header_; }\n> > \tstd::vector &data() { return data_; }\n> > -\tstd::vector &fds() { return fds_; }\n> > +\tstd::vector &fds() { return fds_; }\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> > private:\n> > \tHeader header_;\n> > \tstd::vector data_;\n> > -\tstd::vector fds_;\n> > +\tstd::vector fds_;\n> > };\n> > class IPCPipe\n> > diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n> > index fb941e6b..91471f52 100644\n> > --- a/src/libcamera/ipa_data_serializer.cpp\n> > +++ b/src/libcamera/ipa_data_serializer.cpp\n> > @@ -7,6 +7,8 @@\n> > #include \"libcamera/internal/ipa_data_serializer.h\"\n> > +#include \n> > +\n> > #include \n> > /**\n> > @@ -141,7 +143,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 +164,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 +189,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\\\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 +217,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 +226,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 +251,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 +278,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 +288,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 +307,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 +407,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 +417,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 +431,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 +493,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 +503,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 +524,45 @@ 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> > +\tstd::vector dataVec;\n> > +\tstd::vector fdVec;\n> > +\n> > +\t/*\n> > +\t * Store as uint32_t to prepare for conversion from validity flag\n> > +\t * to index, and for alignment\n> > +\t */\n> > +\tappendPOD(dataVec, data.isValid());\n> > +\n> > \tif (data.isValid())\n> > -\t\tfdVec.push_back(data.fd());\n> > +\t\tfdVec.push_back(data);\n> > +\n> > \treturn { dataVec, fdVec };\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> > +\tASSERT(std::distance(dataBegin, dataEnd) >= 4);\n> > -\tbool valid = !!(*dataBegin);\n> > +\tuint32_t valid = readPOD(dataBegin, 0, dataEnd);\n> > \tASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));\n> > -\treturn valid ? FileDescriptor(*fdsBegin) : FileDescriptor();\n> > +\treturn valid ? *fdsBegin : FileDescriptor();\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 +575,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> > \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,13 +598,13 @@ 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> > -\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 1,\n> > +\tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n> > \t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n> > \tret.length = readPOD(dataBegin, 1, dataEnd);\n> > @@ -604,7 +614,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> > @@ -104,7 +108,9 @@ IPCUnixSocket::Payload IPCMessage::payload() const\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> > \treturn payload;\n> > }\n> > diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp\n> > index bf7e34e2..eca19a66 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> > \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> > \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> > \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> > \t\tstd::vector buf;\n> > -\t\tstd::vector fds;\n> > +\t\tstd::vector fds;\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> > \t\tstd::vector buf;\n> > -\t\tstd::vector fds;\n> > +\t\tstd::vector fds;\n> > \t\tif (testMapSerdes(mapUintStr) != TestPass)\n> > \t\t\treturn TestFail;\n> > @@ -359,7 +359,7 @@ private:\n> > \t\tstd::string strEmpty = \"\";\n> > \t\tstd::vector buf;\n> > -\t\tstd::vector fds;\n> > +\t\tstd::vector fds;\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 c222f5f2..1979e68f 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> > \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..b8ef8e7b 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> > @@ -104,9 +104,9 @@\n> > \t\tdataSize -= {{field_size}};\n> > \t{%- endif %}\n> > {% elif field|is_fd %}\n> > -\t{%- set field_size = 1 %}\n> > +\t{%- set field_size = 4 %}\n> > \t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> > -\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs);\n> > +\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs);\n> > \t{%- if not loop.last %}\n> > \t\tm += {{field_size}};\n> > \t\tdataSize -= {{field_size}};\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> > \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 05C63BD87D\n\tfor ;\n\tThu, 19 Aug 2021 07:13:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E0ED688A3;\n\tThu, 19 Aug 2021 09:13:50 +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 959476888E\n\tfor ;\n\tThu, 19 Aug 2021 09:13:48 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C14E02A8;\n\tThu, 19 Aug 2021 09:13:46 +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=\"hQGRYtF4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629357228;\n\tbh=g+gba42tNDXH3gmrv5rWS7lMVZDPW4WrMVCJvzwR89o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hQGRYtF43gMGf6fcBAzYL7SrRyuLXd3TJlxvPFw0R0Pj3uTBjUcUS1eKi6nAvwpSX\n\t4XB7m5cYYRiNDzY1ujFYQUywo7fZy3Ab8GTUoZCtDIWhunCbseH5oYDWf7GmP1nddh\n\tj5KWNTM3Xjiu9oJX+MKBGs5IsZMBZXAYIQB1I3i8=","Date":"Thu, 19 Aug 2021 16:13:40 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain ","Message-ID":"<20210819071340.GM1733965@pyrite.rasen.tech>","References":"<20210817044633.2061596-1-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3] 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\" "}}]