Removed usage of variable length arrays, fixed memory leaks#2400
Conversation
* ACE/ace/Svc_Conf_Lexer.cpp:
WalkthroughThe code in Changes
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 (
|
|
clang20 does always give a warning now when a variable length array is used |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
ACE/ace/Svc_Conf_Lexer.cpp (1)
20-22:⚠️ Potential issue
<memory>must be included unconditionally
std::unique_ptr/std::make_uniqueare now used for all compilers, but<memory>is only included when__GNUG__is not defined.
With GCC/Clang this header may be transitively included today, yet that is an implementation detail and breaks clean‐room builds or older libstdc++ versions.-#if !defined (__GNUG__) -# include <memory> -#endif +#include <memory> // needed for std::unique_ptr / std::make_unique
🧹 Nitpick comments (1)
ACE/ace/Svc_Conf_Lexer.cpp (1)
411-416: Avoid per-token heap allocation in the hot pathReplacing the VLA with
std::unique_ptr<char[]> str_bufp = std::make_unique<char[]>(size);creates a heap allocation for every identifier scanned.
For large service config files this is a measurable regression compared to the former stack allocation.Optional, but recommended:
- Reserve a small fixed stack buffer (e.g.
char local[256]) and fall back tostd::unique_ptronly whensize > sizeof local.- Alternatively, use
std::string_viewand do the keyword comparisons directly on the input buffer, eliminating the copy completely.No functional change – purely a performance / allocation-pressure optimisation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ACE/ace/Svc_Conf_Lexer.cpp(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: alpine-3.18
- GitHub Check: ubuntu-24.04 g++
- GitHub Check: ubuntu-24.04 g++-10 wchar
- GitHub Check: ubuntu-24.04 g++-10 CORBA/e compact
- GitHub Check: ubuntu-24.04 g++-10 CORBA/e micro
- GitHub Check: ubuntu-24.04 g++-10 ACE for TAO
- GitHub Check: ubuntu-24.04 clang++-14
- GitHub Check: VS2022Debug64Cxx17
- GitHub Check: ubuntu-24.04 g++-12
- GitHub Check: ubuntu-22.04 g++-10
- GitHub Check: ubuntu-24.04 g++-11
- GitHub Check: ubuntu-24.04 g++-14
- GitHub Check: ubuntu-22.04 clang++-13
- GitHub Check: ubuntu-22.04 g++-9
- GitHub Check: ubuntu-24.04 clang++-16
- GitHub Check: ubuntu-24.04 g++-13
- GitHub Check: ubuntu-22.04 clang++-12
- GitHub Check: windows-2022 vs2022
- GitHub Check: ubuntu-24.04 clang++-15
- GitHub Check: MINGW64-gcc
🔇 Additional comments (1)
ACE/ace/Svc_Conf_Lexer.cpp (1)
411-416:std::make_uniquerequires C++14ACE still supports C++11 tool-chains in several environments.
std::make_uniqueis C++14; compilation will fail with C++11.
If C++14 is not guaranteed, fall back to explicit construction:-std::unique_ptr<char[]> str_bufp = std::make_unique<char[]>(size); +std::unique_ptr<char[]> str_bufp(new char[size]);Would you confirm the minimum C++ standard ACE targets for this branch?
* ACE/ace/Svc_Conf_Lexer.cpp:
|
@coderabbitai full review |
|
|
I just wanted to make a simple change to get rid of a warning with a compiler I am testing, introducing |
Summary by CodeRabbit