| Message ID | 20210804113529.418162-1-paul.elder@ideasonboard.com |
|---|---|
| State | New |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul On 8/4/21 5:05 PM, Paul Elder wrote: > Add tests to test IPADataSerializer serializing FileDescriptors. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/file_descriptor.h | 1 + > src/libcamera/file_descriptor.cpp | 9 ++++ > .../ipa_data_serializer_test.cpp | 43 +++++++++++++++++++ > 3 files changed, 53 insertions(+) > > diff --git a/include/libcamera/file_descriptor.h b/include/libcamera/file_descriptor.h > index d514aac7..8f1d6f21 100644 > --- a/include/libcamera/file_descriptor.h > +++ b/include/libcamera/file_descriptor.h > @@ -22,6 +22,7 @@ public: > > FileDescriptor &operator=(const FileDescriptor &other); > FileDescriptor &operator=(FileDescriptor &&other); > + bool operator==(const FileDescriptor &other) const; > > bool isValid() const { return fd_ != nullptr; } > int fd() const { return fd_ ? fd_->fd() : -1; } > diff --git a/src/libcamera/file_descriptor.cpp b/src/libcamera/file_descriptor.cpp > index 638b3bbe..f1394153 100644 > --- a/src/libcamera/file_descriptor.cpp > +++ b/src/libcamera/file_descriptor.cpp > @@ -191,6 +191,15 @@ FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other) > return *this; > } > > +/** > + * \brief Compare FileDescriptor instances for equality > + * \return True if the underlying file descriptor is equal, false otherwise > + */ > +bool FileDescriptor::operator==(const FileDescriptor &other) const > +{ > + return this->fd() == other.fd(); In context of testVectorSerdes() which serdes a vector of FileDescriptors, this makes sense to me. Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > +} > + > /** > * \fn FileDescriptor::isValid() > * \brief Check if the FileDescriptor instance is valid > diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp > index 1e8f7e21..9c8ff69c 100644 > --- a/test/serialization/ipa_data_serializer_test.cpp > +++ b/test/serialization/ipa_data_serializer_test.cpp > @@ -11,6 +11,7 @@ > #include <iostream> > #include <limits> > #include <stdlib.h> > +#include <sys/mman.h> > #include <sys/stat.h> > #include <sys/types.h> > #include <tuple> > @@ -144,6 +145,10 @@ protected: > if (ret != TestPass) > return ret; > > + ret = testFd(); > + if (ret != TestPass) > + return ret; > + > return TestPass; > } > > @@ -435,6 +440,44 @@ private: > > return TestPass; > } > + > + int testFd() > + { > + /* Test serdes of single fd */ > + int memfd = memfd_create("test", 0); > + if (memfd < 0) { > + cerr << "Failed to create memfd" << endl; > + return TestFail; > + } > + > + FileDescriptor fd = FileDescriptor(std::move(memfd)); > + if (memfd != -1) { > + cerr << "FileDescriptor move constructor failed" << endl; > + return TestFail; > + } > + > + if (testPodSerdes(fd) != TestPass) > + return TestFail; > + > + /* Test serdes of vector of fds */ > + std::vector<FileDescriptor> vecFds; > + > + for (unsigned int i = 0; i < 10; i++) { > + std::string name = "test" + std::to_string(i); > + int mfd = memfd_create(name.c_str(), 0); > + if (mfd < 0) > + return TestFail; > + > + vecFds.push_back(FileDescriptor(std::move(mfd))); > + if (mfd != -1) > + return TestFail; > + } > + > + if (testVectorSerdes(vecFds) != TestPass) > + return TestFail; > + > + return TestPass; > + } > }; > > TEST_REGISTER(IPADataSerializerTest)
Hi Paul, Thank you for the patch. On Wed, Aug 04, 2021 at 08:35:29PM +0900, Paul Elder wrote: > Add tests to test IPADataSerializer serializing FileDescriptors. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/file_descriptor.h | 1 + > src/libcamera/file_descriptor.cpp | 9 ++++ > .../ipa_data_serializer_test.cpp | 43 +++++++++++++++++++ > 3 files changed, 53 insertions(+) > > diff --git a/include/libcamera/file_descriptor.h b/include/libcamera/file_descriptor.h > index d514aac7..8f1d6f21 100644 > --- a/include/libcamera/file_descriptor.h > +++ b/include/libcamera/file_descriptor.h > @@ -22,6 +22,7 @@ public: > > FileDescriptor &operator=(const FileDescriptor &other); > FileDescriptor &operator=(FileDescriptor &&other); > + bool operator==(const FileDescriptor &other) const; > > bool isValid() const { return fd_ != nullptr; } > int fd() const { return fd_ ? fd_->fd() : -1; } > diff --git a/src/libcamera/file_descriptor.cpp b/src/libcamera/file_descriptor.cpp > index 638b3bbe..f1394153 100644 > --- a/src/libcamera/file_descriptor.cpp > +++ b/src/libcamera/file_descriptor.cpp > @@ -191,6 +191,15 @@ FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other) > return *this; > } > > +/** > + * \brief Compare FileDescriptor instances for equality > + * \return True if the underlying file descriptor is equal, false otherwise > + */ > +bool FileDescriptor::operator==(const FileDescriptor &other) const > +{ > + return this->fd() == other.fd(); > +} > + This could alternatively be defined in test/serialization/ipa_data_serializer_test.cpp, like we do for ControlInfoMap. It would allow us to defer the decision on what equality of file descriptors means in general. > /** > * \fn FileDescriptor::isValid() > * \brief Check if the FileDescriptor instance is valid > diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp > index 1e8f7e21..9c8ff69c 100644 > --- a/test/serialization/ipa_data_serializer_test.cpp > +++ b/test/serialization/ipa_data_serializer_test.cpp > @@ -11,6 +11,7 @@ > #include <iostream> > #include <limits> > #include <stdlib.h> > +#include <sys/mman.h> > #include <sys/stat.h> > #include <sys/types.h> > #include <tuple> > @@ -144,6 +145,10 @@ protected: > if (ret != TestPass) > return ret; > > + ret = testFd(); > + if (ret != TestPass) > + return ret; > + > return TestPass; > } > > @@ -435,6 +440,44 @@ private: > > return TestPass; > } > + > + int testFd() The test currently fail, so it's a good one :-) > + { > + /* Test serdes of single fd */ > + int memfd = memfd_create("test", 0); > + if (memfd < 0) { > + cerr << "Failed to create memfd" << endl; > + return TestFail; > + } > + > + FileDescriptor fd = FileDescriptor(std::move(memfd)); > + if (memfd != -1) { > + cerr << "FileDescriptor move constructor failed" << endl; > + return TestFail; > + } > + > + if (testPodSerdes(fd) != TestPass) > + return TestFail; I highly recommend setting the meson option b_sanitize to "address,undefined": 26/63 libcamera:serialization / ipa_data_serializer_test FAIL 0.11s (exit status 255 or signal 127 SIGinvalid) >>> MALLOC_PERTURB_=209 build/x86-gcc-11.1.0/test/serialization/ipa_data_serializer_test ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― Listing only the last 100 lines from a long log. 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/bits/stl_function.h:386:20: runtime error: load of misaligned address 0x612000001192 for type 'const unsigned int', which requires 4 byte alignment 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ ../../src/libcamera/control_serializer.cpp:363:11: runtime error: member access within misaligned address 0x61200000118e for type 'const struct ipa_controls_header', which requires 4 byte alignment 0x61200000118e: note: pointer points here 68 00 00 00 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc ^ ../../src/libcamera/control_serializer.cpp:370:50: runtime error: member access within misaligned address 0x61200000118e for type 'const struct ipa_controls_header', which requires 4 byte alignment 0x61200000118e: note: pointer points here 68 00 00 00 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc ^ ../../src/libcamera/control_serializer.cpp:371:49: runtime error: member access within misaligned address 0x61200000118e for type 'const struct ipa_controls_header', which requires 4 byte alignment 0x61200000118e: note: pointer points here 68 00 00 00 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc ^ ../../src/libcamera/control_serializer.cpp:371:61: runtime error: member access within misaligned address 0x61200000118e for type 'const struct ipa_controls_header', which requires 4 byte alignment 0x61200000118e: note: pointer points here 68 00 00 00 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc ^ ../../src/libcamera/control_serializer.cpp:380:36: runtime error: member access within misaligned address 0x61200000118e for type 'const struct ipa_controls_header', which requires 4 byte alignment 0x61200000118e: note: pointer points here 68 00 00 00 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc ^ ../../src/libcamera/control_serializer.cpp:389:54: runtime error: member access within misaligned address 0x6120000011ae for type 'const struct ipa_control_info_entry', which requires 4 byte alignment 0x6120000011ae: note: pointer points here 93 55 00 00 09 00 00 00 05 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 05 00 00 00 08 00 00 00 ^ ../../src/libcamera/control_serializer.cpp:394:63: runtime error: member access within misaligned address 0x6120000011ae for type 'const struct ipa_control_info_entry', which requires 4 byte alignment 0x6120000011ae: note: pointer points here 93 55 00 00 09 00 00 00 05 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 05 00 00 00 08 00 00 00 ^ ../../src/libcamera/control_serializer.cpp:394:63: runtime error: reference binding to misaligned address 0x6120000011ae for type 'const unsigned int', which requires 4 byte alignment 0x6120000011ae: note: pointer points here 93 55 00 00 09 00 00 00 05 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 05 00 00 00 08 00 00 00 ^ ../../src/libcamera/control_serializer.cpp:394:63: runtime error: member access within misaligned address 0x6120000011ae for type 'const struct ipa_control_info_entry', which requires 4 byte alignment 0x6120000011ae: note: pointer points here 93 55 00 00 09 00 00 00 05 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 05 00 00 00 08 00 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/bits/unique_ptr.h:962:57: runtime error: reference binding to misaligned address 0x6120000011ae for type 'const type', which requires 4 byte alignment 0x6120000011ae: note: pointer points here 93 55 00 00 09 00 00 00 05 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 05 00 00 00 08 00 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/bits/move.h:78:36: runtime error: reference binding to misaligned address 0x6120000011ae for type 'const unsigned int', which requires 4 byte alignment 0x6120000011ae: note: pointer points here 93 55 00 00 09 00 00 00 05 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 05 00 00 00 08 00 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/bits/unique_ptr.h:962:69: runtime error: load of misaligned address 0x6120000011ae for type 'const unsigned int', which requires 4 byte alignment 0x6120000011ae: note: pointer points here 93 55 00 00 09 00 00 00 05 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 05 00 00 00 08 00 00 00 ^ ../../src/libcamera/control_serializer.cpp:396:14: runtime error: member access within misaligned address 0x6120000011ae for type 'const struct ipa_control_info_entry', which requires 4 byte alignment 0x6120000011ae: note: pointer points here 93 55 00 00 09 00 00 00 05 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 05 00 00 00 08 00 00 00 ^ ../../src/libcamera/control_serializer.cpp:412:39: runtime error: member access within misaligned address 0x61200000118e for type 'const struct ipa_controls_header', which requires 4 byte alignment 0x61200000118e: note: pointer points here 68 00 00 00 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc ^ ../../src/libcamera/control_serializer.cpp:412:39: runtime error: reference binding to misaligned address 0x612000001192 for type 'const key_type', which requires 4 byte alignment 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ ../../src/libcamera/control_serializer.cpp:412:39: runtime error: member access within misaligned address 0x61200000118e for type 'const struct ipa_controls_header', which requires 4 byte alignment 0x61200000118e: note: pointer points here 68 00 00 00 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/bits/stl_map.h:1259:32: runtime error: reference binding to misaligned address 0x612000001192 for type 'const key_type', which requires 4 byte alignment 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/bits/stl_tree.h:1270:30: runtime error: reference binding to misaligned address 0x612000001192 for type 'const unsigned int', which requires 4 byte alignment 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/bits/stl_map.h:502:15: runtime error: reference binding to misaligned address 0x612000001192 for type 'const unsigned int', which requires 4 byte alignment 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/tuple:444:49: runtime error: reference binding to misaligned address 0x612000001192 for type 'const type', which requires 4 byte alignment 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/tuple:1771:35: runtime error: reference binding to misaligned address 0x612000001192 for type 'const type', which requires 4 byte alignment 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ /usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/tuple:1771:9: runtime error: load of misaligned address 0x612000001192 for type 'const unsigned int', which requires 4 byte alignment 0x612000001192: note: pointer points here 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc 93 55 00 00 ^ ../../src/libcamera/control_serializer.cpp:413:31: runtime error: member access within misaligned address 0x61200000118e for type 'const struct ipa_controls_header', which requires 4 byte alignment 0x61200000118e: note: pointer points here 68 00 00 00 01 00 00 00 04 00 00 00 03 00 00 00 68 00 00 00 50 00 00 00 fe 7f 00 00 8c c1 9e bc ^ Deserialized libcamera::FileDescriptor doesn't match original > + > + /* Test serdes of vector of fds */ > + std::vector<FileDescriptor> vecFds; > + > + for (unsigned int i = 0; i < 10; i++) { > + std::string name = "test" + std::to_string(i); > + int mfd = memfd_create(name.c_str(), 0); > + if (mfd < 0) > + return TestFail; > + > + vecFds.push_back(FileDescriptor(std::move(mfd))); > + if (mfd != -1) > + return TestFail; > + } > + > + if (testVectorSerdes(vecFds) != TestPass) > + return TestFail; The test looks good, but I'd like to have a fix before merging it. > + > + return TestPass; > + } > }; > > TEST_REGISTER(IPADataSerializerTest)
diff --git a/include/libcamera/file_descriptor.h b/include/libcamera/file_descriptor.h index d514aac7..8f1d6f21 100644 --- a/include/libcamera/file_descriptor.h +++ b/include/libcamera/file_descriptor.h @@ -22,6 +22,7 @@ public: FileDescriptor &operator=(const FileDescriptor &other); FileDescriptor &operator=(FileDescriptor &&other); + bool operator==(const FileDescriptor &other) const; bool isValid() const { return fd_ != nullptr; } int fd() const { return fd_ ? fd_->fd() : -1; } diff --git a/src/libcamera/file_descriptor.cpp b/src/libcamera/file_descriptor.cpp index 638b3bbe..f1394153 100644 --- a/src/libcamera/file_descriptor.cpp +++ b/src/libcamera/file_descriptor.cpp @@ -191,6 +191,15 @@ FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other) return *this; } +/** + * \brief Compare FileDescriptor instances for equality + * \return True if the underlying file descriptor is equal, false otherwise + */ +bool FileDescriptor::operator==(const FileDescriptor &other) const +{ + return this->fd() == other.fd(); +} + /** * \fn FileDescriptor::isValid() * \brief Check if the FileDescriptor instance is valid diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp index 1e8f7e21..9c8ff69c 100644 --- a/test/serialization/ipa_data_serializer_test.cpp +++ b/test/serialization/ipa_data_serializer_test.cpp @@ -11,6 +11,7 @@ #include <iostream> #include <limits> #include <stdlib.h> +#include <sys/mman.h> #include <sys/stat.h> #include <sys/types.h> #include <tuple> @@ -144,6 +145,10 @@ protected: if (ret != TestPass) return ret; + ret = testFd(); + if (ret != TestPass) + return ret; + return TestPass; } @@ -435,6 +440,44 @@ private: return TestPass; } + + int testFd() + { + /* Test serdes of single fd */ + int memfd = memfd_create("test", 0); + if (memfd < 0) { + cerr << "Failed to create memfd" << endl; + return TestFail; + } + + FileDescriptor fd = FileDescriptor(std::move(memfd)); + if (memfd != -1) { + cerr << "FileDescriptor move constructor failed" << endl; + return TestFail; + } + + if (testPodSerdes(fd) != TestPass) + return TestFail; + + /* Test serdes of vector of fds */ + std::vector<FileDescriptor> vecFds; + + for (unsigned int i = 0; i < 10; i++) { + std::string name = "test" + std::to_string(i); + int mfd = memfd_create(name.c_str(), 0); + if (mfd < 0) + return TestFail; + + vecFds.push_back(FileDescriptor(std::move(mfd))); + if (mfd != -1) + return TestFail; + } + + if (testVectorSerdes(vecFds) != TestPass) + return TestFail; + + return TestPass; + } }; TEST_REGISTER(IPADataSerializerTest)
Add tests to test IPADataSerializer serializing FileDescriptors. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- include/libcamera/file_descriptor.h | 1 + src/libcamera/file_descriptor.cpp | 9 ++++ .../ipa_data_serializer_test.cpp | 43 +++++++++++++++++++ 3 files changed, 53 insertions(+)