cpp-libp2p icon indicating copy to clipboard operation
cpp-libp2p copied to clipboard

Fix code quality issues and improve portability

Open alienx5499 opened this issue 5 months ago • 2 comments

The cpp-libp2p repository had several code quality issues that were causing compiler warnings and potential portability problems:

  1. Variable Length Arrays (VLA) - Non-standard C++ extension causing compiler warnings
  2. Deprecated lambda capture syntax - Using [=] with this is deprecated in C++20
  3. Deprecated OpenSSL function - BN_pseudo_rand is deprecated and should be replaced

Root Cause

  • VLA warnings: Code was using C-style variable length arrays which are not part of the C++ standard and cause portability issues
  • Lambda capture warnings: Modern C++ standards deprecate implicit capture of this with [=]
  • OpenSSL deprecation: BN_pseudo_rand function has been deprecated in favor of BN_rand

Solution

Updated the code to use modern, standard C++ practices and current OpenSSL APIs:

src/security/tls/tls_details.cpp

  • Replaced variable length arrays with std::vector<uint8_t> for better portability
  • Updated BN_pseudo_rand to BN_rand for current OpenSSL compatibility
  • Maintained the same functionality while improving code quality

src/transport/tcp/tcp_transport.cpp

  • Fixed deprecated lambda capture syntax by removing [=] and using explicit captures
  • Improved C++20 compatibility

Changes Made

src/security/tls/tls_details.cpp

// Before: Variable length array (non-standard)
uint8_t buf[msg_len];  // NOLINT

// After: Standard C++ vector
std::vector<uint8_t> buf(msg_len);
// Before: Deprecated OpenSSL function
if (BN_pseudo_rand(bn, 64, 0, 0) == 0) {

// After: Current OpenSSL function
if (BN_rand(bn, 64, 0, 0) == 0) {

src/transport/tcp/tcp_transport.cpp

// Before: Deprecated lambda capture
[=, self{shared_from_this()}, ...]

// After: Explicit capture
[self{shared_from_this()}, ...]

Testing

  • ✅ Clean build successful
  • ✅ No VLA warnings generated
  • ✅ No deprecated capture warnings
  • ✅ No deprecated OpenSSL function warnings
  • ✅ All functionality preserved

Before/After

Before: Build generated warnings:

warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
warning: implicit capture of 'this' with a capture default of '=' is deprecated [-Wdeprecated-this-capture]

After: Clean build with no warnings

Impact

  • Code Quality: Improved adherence to C++ standards
  • Portability: Better compatibility across different compilers and platforms
  • Maintainability: Removed deprecated function usage
  • Future Compatibility: Better support for upcoming C++ standards
  • No Breaking Changes: All functionality preserved

Related Issues

This fix addresses compiler warnings and improves code quality without changing the library's behavior or API.

Notes

  • No changes to the actual library functionality or API
  • Only affects internal implementation details
  • Improves code quality and reduces compiler warnings
  • Follows modern C++ best practices
  • Maintains backward compatibility

alienx5499 avatar Aug 30 '25 07:08 alienx5499

@turuslan Could you please review this PR when you get a chance?

alienx5499 avatar Sep 03 '25 14:09 alienx5499

Thank you for your contribution

turuslan avatar Sep 03 '25 15:09 turuslan