Updated ACE tests for Android NDK's Clang#2430
Conversation
Avoids errors and warnings on this platform
WalkthroughThe changes primarily refactor and clean up several test files by adjusting variable scope, improving conditional compilation, updating loop constructs, and correcting return values. Some unused variables and constants are removed, and explicit constructors are added for clarity. No major logic or control flow changes are introduced. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
ACE/tests/Compiler_Features_16_Test.cpp(1 hunks)ACE/tests/MM_Shared_Memory_Test.cpp(2 hunks)ACE/tests/Multicast_Interfaces_Test.cpp(1 hunks)ACE/tests/OS_Test.cpp(1 hunks)ACE/tests/Reactor_Remove_Resume_Test_Dev_Poll.cpp(1 hunks)ACE/tests/Recursive_Condition_Test.cpp(2 hunks)ACE/tests/SOCK_Send_Recv_Test_IPV6.cpp(1 hunks)ACE/tests/STL_algorithm_Test_T.cpp(1 hunks)ACE/tests/Sendfile_Test.cpp(0 hunks)ACE/tests/Thread_Pool_Test.cpp(2 hunks)ACE/tests/randomize.h(1 hunks)
💤 Files with no reviewable changes (1)
- ACE/tests/Sendfile_Test.cpp
🧰 Additional context used
🧠 Learnings (4)
ACE/tests/Multicast_Interfaces_Test.cpp (3)
Learnt from: jwillemsen
PR: #870
File: ACE/ace/UNIX_Addr.inl:39-39
Timestamp: 2025-02-11T10:00:08.023Z
Learning: In ACE_UNIX_Addr's equality operator, the comparison length should be sizeof(sun_path) - 1 to exclude the null terminator, not i. For Linux abstract paths, i is used to skip the first character (i=1) when both paths are empty, but the -1 is still needed to handle the null terminator correctly.
Learnt from: jwillemsen
PR: #870
File: ACE/ace/UNIX_Addr.inl:39-39
Timestamp: 2025-02-11T10:00:08.023Z
Learning: In ACE_UNIX_Addr's equality operator, the comparison length should be sizeof(sun_path) - 1 to exclude the null terminator. For Linux abstract paths, while i=1 is used to skip the first character, using i instead of -1 in the length calculation would be incorrect as it would either include the null terminator (for regular paths) or miss the last valid character (for abstract paths).
Learnt from: likema
PR: #870
File: ACE/ace/UNIX_Addr.cpp:0-0
Timestamp: 2025-02-07T16:15:05.367Z
Learning: The ACE_UNIX_Addr class in ACE/ace/UNIX_Addr.cpp supports abstract paths in Linux, where paths starting with '@' are treated specially by having their first character replaced with a null terminator in the sun_path field.
ACE/tests/SOCK_Send_Recv_Test_IPV6.cpp (4)
Learnt from: jwillemsen
PR: #870
File: ACE/ace/UNIX_Addr.inl:39-39
Timestamp: 2025-02-11T10:00:08.023Z
Learning: In ACE_UNIX_Addr's equality operator, the comparison length should be sizeof(sun_path) - 1 to exclude the null terminator, not i. For Linux abstract paths, i is used to skip the first character (i=1) when both paths are empty, but the -1 is still needed to handle the null terminator correctly.
Learnt from: jwillemsen
PR: #870
File: ACE/ace/UNIX_Addr.inl:39-39
Timestamp: 2025-02-11T10:00:08.023Z
Learning: In ACE_UNIX_Addr's equality operator, the comparison length should be sizeof(sun_path) - 1 to exclude the null terminator. For Linux abstract paths, while i=1 is used to skip the first character, using i instead of -1 in the length calculation would be incorrect as it would either include the null terminator (for regular paths) or miss the last valid character (for abstract paths).
Learnt from: likema
PR: #870
File: ACE/ace/UNIX_Addr.cpp:0-0
Timestamp: 2025-02-07T16:15:05.367Z
Learning: The ACE_UNIX_Addr class in ACE/ace/UNIX_Addr.cpp supports abstract paths in Linux, where paths starting with '@' are treated specially by having their first character replaced with a null terminator in the sun_path field.
Learnt from: likema
PR: #870
File: ACE/ace/UNIX_Addr.cpp:0-0
Timestamp: 2025-02-07T17:43:40.156Z
Learning: In ACE_UNIX_Addr::addr_to_string, when handling abstract paths on Linux (paths starting with '@'), the function returns -1 if the output buffer length is 1, as it needs at least 2 characters to store the '@' prefix and a null terminator.
ACE/tests/MM_Shared_Memory_Test.cpp (1)
Learnt from: likema
PR: #870
File: ACE/ace/UNIX_Addr.cpp:0-0
Timestamp: 2025-02-07T17:43:40.156Z
Learning: In ACE_UNIX_Addr::addr_to_string, when handling abstract paths on Linux (paths starting with '@'), the function returns -1 if the output buffer length is 1, as it needs at least 2 characters to store the '@' prefix and a null terminator.
ACE/tests/OS_Test.cpp (2)
Learnt from: likema
PR: #870
File: ACE/ace/UNIX_Addr.cpp:0-0
Timestamp: 2025-02-07T17:43:40.156Z
Learning: In ACE_UNIX_Addr::addr_to_string, when handling abstract paths on Linux (paths starting with '@'), the function returns -1 if the output buffer length is 1, as it needs at least 2 characters to store the '@' prefix and a null terminator.
Learnt from: jwillemsen
PR: #870
File: ACE/ace/UNIX_Addr.inl:39-39
Timestamp: 2025-02-11T10:00:08.023Z
Learning: In ACE_UNIX_Addr's equality operator, the comparison length should be sizeof(sun_path) - 1 to exclude the null terminator, not i. For Linux abstract paths, i is used to skip the first character (i=1) when both paths are empty, but the -1 is still needed to handle the null terminator correctly.
🔇 Additional comments (11)
ACE/tests/STL_algorithm_Test_T.cpp (1)
17-17: LGTM! Explicit default copy constructor improves clarity.Adding the explicit default copy constructor is a good practice that enhances code clarity and ensures consistent behavior across different compilers, including Android NDK's Clang.
ACE/tests/Compiler_Features_16_Test.cpp (2)
26-26: Fix namespace closing syntax.Correcting the namespace closing from
};to}fixes a syntax issue that could cause compiler warnings or errors.
28-28: Improved function signature formatting.The reformatted function signature enhances readability by placing it on a single line with consistent spacing.
ACE/tests/OS_Test.cpp (1)
1211-1211: Fix return value to properly report test failures.Returning
retvalinstead of always returning0ensures that the accumulated error count is properly propagated to the caller, allowing test failures to be correctly detected.ACE/tests/Reactor_Remove_Resume_Test_Dev_Poll.cpp (1)
378-378: Simplified struct design by removing std::function inheritance.Removing the inheritance from
std::function<void(reactor_factory_type)>simplifies the design while maintaining the same functionality through theoperator()overload. This change improves compiler compatibility and reduces template complexity.ACE/tests/randomize.h (1)
58-58: Improve numerical precision consistency.Changing from
1.0fto1.0ensures the division operation uses double precision consistently, matching thedouble const coefficient_member type and avoiding potential mixed-precision arithmetic warnings.ACE/tests/Multicast_Interfaces_Test.cpp (1)
108-109: LGTM! Proper handling of unused parameter.The addition of the
#elseblock withACE_UNUSED_ARG(names)correctly suppresses unused parameter warnings on platforms that don't support either Windows-specific APIs orgetifaddrs. This aligns well with the PR objective of avoiding warnings with Android NDK's Clang.ACE/tests/Thread_Pool_Test.cpp (2)
208-208: Good refactoring for code clarity.Replacing the
forloop withwhile (true)makes the infinite loop intent more explicit and removes the unused loop increment variable. This is cleaner and may help avoid compiler warnings about unused variables.
295-295: Consistent improvement with the previous change.Same beneficial refactoring as in
test_queue_deactivation_shutdown()- thewhile (true)construct is more explicit about the infinite loop intent and removes the unused loop increment variable.ACE/tests/SOCK_Send_Recv_Test_IPV6.cpp (1)
35-35: Proper conditional compilation for IPv6-specific code.Adding the
#if defined (ACE_HAS_IPV6)guard around the test constants andclientfunction is appropriate since they're only used in IPv6-specific tests. This prevents compilation issues on platforms without IPv6 support and aligns with the existing conditional compilation pattern in the file.ACE/tests/MM_Shared_Memory_Test.cpp (1)
64-67: Good conditional compilation for mktemp-dependent code.Moving the static variables inside the
#ifndef ACE_DISABLE_MKTEMPguard is appropriate since they're only used in the shared memory test logic that depends onmktempfunctionality. This prevents unused variable warnings whenACE_DISABLE_MKTEMPis defined and aligns with the existing conditional compilation inrun_main().Also applies to: 198-198
2 participants
Avoids errors and warnings on this platform
Summary by CodeRabbit
Bug Fixes
Refactor
New Features
Element_Countertemplate class.