Remove clang-tidy checks in source code #2
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| << ">1 shared pointer references to " | ||
| "it."); | ||
| // NOLINTEND(bugprone-branch-clone) | ||
| if (m_managed_buffer_ptr.use_count() == 0) { |
There was a problem hiding this comment.
if with identical then and else branches
| #define ASSERTION_TAIL \ | ||
| << debug_output(fileline) << (_shall_stop_after_assertion = true,""), \ | ||
| assert(!_shall_stop_after_assertion ) | ||
| #define AEQ(v1,v2) \ |
There was a problem hiding this comment.
function-like macro AEQ used; consider a constexpr template function
| ASSERT_EQ(v1,v2) ASSERTION_TAIL; \ | ||
| ++n_assertions; \ | ||
| } while(0) | ||
| #define ANE(v1,v2) \ |
There was a problem hiding this comment.
function-like macro ANE used; consider a constexpr template function
| #define CHECK_SIZES(POSITION, CAPACITY) \ | ||
| check_sizes(FILELINE(), debug_output, mbs, buffer_size, POSITION, CAPACITY) | ||
| // NOLINTEND(cppcoreguidelines-macro-usage) | ||
| #define CHECK_SIZES(POSITION,CAPACITY) \ |
There was a problem hiding this comment.
function-like macro CHECK_SIZES used; consider a constexpr template function
| #define NAMED_THD_STAGE_GUARD(name, thd, new_stage) \ | ||
| raii::Thread_stage_guard name { \ | ||
| (thd), (new_stage), __func__, __FILE__, __LINE__ \ | ||
| #define NAMED_THD_STAGE_GUARD(name, thd, new_stage) \ |
There was a problem hiding this comment.
function-like macro NAMED_THD_STAGE_GUARD used; consider a constexpr template function
| NAMED_THD_STAGE_GUARD(_thread_stage_guard_##new_stage, (thd), (new_stage)) | ||
|
|
||
| // NOLINTEND(cppcoreguidelines-macro-usage) | ||
| #define THD_STAGE_GUARD(thd,new_stage) \ |
There was a problem hiding this comment.
function-like macro THD_STAGE_GUARD used; consider a constexpr template function
| @@ -344,7 +337,7 @@ class Payload_event_buffer_istream { | |||
| /// Grow calculator for the Managed_buffer. | |||
| Grow_calculator_t m_grow_calculator; | |||
| /// Default buffer size for the Managed_buffer. | |||
| Size_t m_default_buffer_size; | |||
| Size_t m_default_buffer_size; | |||
There was a problem hiding this comment.
constructor does not initialize these fields: m_default_buffer_size
Suggested change
| Size_t m_default_buffer_size; | |
| Size_t m_default_buffer_size{}; |
| @@ -363,8 +363,7 @@ class PayloadEventBufferStreamTest { | |||
| // "nolint": as a general rule, malloc should not be used, so | |||
| // clang-tidy warns about it. But this is an allocator so it is | |||
| // appropriate to use malloc and therefore we suppress the check. | |||
| // NOLINTNEXTLINE(cppcoreguidelines-no-malloc) | |||
| return std::malloc(n); | |||
| return std::malloc(n); | |||
There was a problem hiding this comment.
do not manage memory manually; consider a container or a smart pointer
venkatesh-prasad-v
force-pushed
the
8.0
branch
from
d110c79
to
7d4d1e2
Compare
venkatesh-prasad-v
pushed a commit
that referenced
this pull request
May 17, 2024
cache [#2] This is second patch, solving the problem of ineffiecent cache invalidation when invalidating a table which is known to be invalid but unknown if it is in the cache or not. Problem: Currently the only way to invalidate a table in the NdbApi dictionary cache is to open the table and then mark it as invalid. In case the table does not exists in the cache, it will still have to be opened and thus fetched fom NDB. This means that in order to get the latest table definition it has to be fetched two times, although the table definition does not already exist in the cache. This is inefficient. Analysis: In order to avoid the double roundtrip there need to be a function which marks the table as invalid only if it exists in the cache. Fix: Implement a NdbApi function that invalidates table by name if it exists in the cache. Replace the old pattern of opening table in order to invalidate it with the new function. The old pattern is still a valid use case for invalidating a table after having worked with it. Change-Id: I20f275f1fed76d991330348bea4ae72548366467
venkatesh-prasad-v
pushed a commit
that referenced
this pull request
Jun 14, 2024
…nt on Windows and posix [#2] The posix version of NdbProcess::start_process assumed the arguments where quoted using " and \ in a way that resembles POSIX sh quoting, and unquoted spaces were treated as argument separators splitting the argument to several. But the Windows version of NdbProcess::start_process did not treat options in the same way. And the Windows C runtime (CRT) parse arguments different from POSIX sh. Note that if program do not use CRT when it may treat the command line in its own way and the quoting done for CRT will mess up the command line. On Windows NdbProcess:start_process should only be used for CRT compatible programs on Windows with respect to argument quoting on command line, or one should make sure given arguments will not trigger unwanted quoting. This may be relevant for ndb_sign_keys and --CA-tool=<batch-file>. Instead this patch change the intention of start_process to pass arguments without modification from caller to the called C programs argument vector in its main entry function. In posix path that is easy, just pass the incoming C strings to execvp. On Windows one need to quote for Windows CRT when composing the command line. Note that the command part of command line have different quoting than the following arguments have. Change-Id: I763530c634d3ea460b24e6e01061bbb5f3321ad4
venkatesh-prasad-v
pushed a commit
that referenced
this pull request
Jun 14, 2024
Problem: Starting ´ndb_mgmd --bind-address´ may potentially cause abnormal program termination in MgmtSrvr destructor when ndb_mgmd restart itself. Core was generated by `ndb_mgmd --defa'. Program terminated with signal SIGABRT, Aborted. #0 0x00007f8ce4066b8f in raise () from /lib64/libc.so.6 #1 0x00007f8ce4039ea5 in abort () from /lib64/libc.so.6 #2 0x00007f8ce40a7d97 in __libc_message () from /lib64/libc.so.6 #3 0x00007f8ce40af08c in malloc_printerr () from /lib64/libc.so.6 #4 0x00007f8ce40b132d in _int_free () from /lib64/libc.so.6 percona#5 0x00000000006e9ffe in MgmtSrvr::~MgmtSrvr (this=0x28de4b0) at mysql/8.0/storage/ndb/src/mgmsrv/MgmtSrvr.cpp: 890 percona#6 0x00000000006ea09e in MgmtSrvr::~MgmtSrvr (this=0x2) at mysql/8.0/ storage/ndb/src/mgmsrv/MgmtSrvr.cpp:849 percona#7 0x0000000000700d94 in mgmd_run () at mysql/8.0/storage/ndb/src/mgmsrv/main.cpp:260 percona#8 0x0000000000700775 in mgmd_main (argc=<optimized out>, argv=0x28041d0) at mysql/8.0/storage/ndb/src/ mgmsrv/main.cpp:479 Analysis: While starting up, the ndb_mgmd will allocate memory for bind_address in order to potentially rewrite the parameter. When ndb_mgmd restart itself the memory will be released and dangling pointer causing double free. Fix: Drop support for bind_address=[::], it is not documented anywhere, is not useful and doesn't work. This means the need to rewrite bind_address is gone and bind_address argument need neither alloc or free. Change-Id: I7797109b9d8391394587188d64d4b1f398887e94
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.