Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PS-9218: Merge MySQL 8.4.0 (fix gcc-14 build) #5309

Merged

Conversation

oleksandr-kachan
Copy link
Contributor

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

@@ -440,7 +440,7 @@ int Ndb::computeHash(Uint32 *retval, const NdbDictionary::Table *table,
while (true) {
if (buf == nullptr) {
bufLen = sumlen;
buf = malloc(bufLen);
buf = calloc(bufLen, 1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-owning-memory ⚠️
assigning newly created gsl::owner<> to non-owner void *

@@ -440,7 +440,7 @@
while (true) {
if (buf == nullptr) {
bufLen = sumlen;
buf = malloc(bufLen);
buf = calloc(bufLen, 1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-analyzer-optin.portability.UnixAPI ⚠️
Call to calloc has an allocation size of 0 bytes

@@ -440,7 +440,7 @@
while (true) {
if (buf == nullptr) {
bufLen = sumlen;
buf = malloc(bufLen);
buf = calloc(bufLen, 1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-no-malloc ⚠️
do not manage memory manually; consider a container or a smart pointer

@percona-ysorokin
Copy link
Collaborator

Please try to compile in RelWithDebInfo mode - there are more warnings.
Also, some of them come from NDB, so enable -DWITH_NDB=ON -DWITH_NDBAPI_EXAMPLES=ON -DWITH_NDB_TEST=ON

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)


if (!mct_log_file) {
printf("Warning: can not open log file (%s): %s. Logging is disabled.\n",
(const char *)mct_log_file_path, (const char *)strerror(errno));
mct_log_file_path.str().c_str(), (const char *)strerror(errno));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ google-readability-casting ⚠️
C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast

@@ -1332,7 +1332,7 @@ void NdbImportUtil::free_rows(RowList &src) {
NdbImportUtil::Blob::Blob() {
m_blobsize = 0;
m_allocsize = 0;
m_data = new uchar[0];
m_data = new uchar[1];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-owning-memory ⚠️
assigning newly created gsl::owner<> to non-owner uchar * (aka unsigned char *)

@@ -1332,7 +1332,7 @@
NdbImportUtil::Blob::Blob() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-prefer-member-initializer ⚠️
m_data should be initialized in a member initializer of the constructor

Suggested change
NdbImportUtil::Blob::Blob() {
NdbImportUtil::Blob::Blob(), m_data(new uchar[1]) {

@@ -1332,7 +1332,7 @@
NdbImportUtil::Blob::Blob() {
m_blobsize = 0;
m_allocsize = 0;
m_data = new uchar[0];
m_data = new uchar[1];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-prefer-member-initializer ⚠️
m_data should be initialized in a member initializer of the constructor

Suggested change
m_data = new uchar[1];

snprintf(mct_log_file_path, FILE_PATH_SIZE, "%s/%s.out.log",
(const char *)tmp_dir, (const char *)test_case_name);
std::stringstream mct_log_file_path;
mct_log_file_path << tmp_dir << "/" << test_case_name << ".out.log";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is C++20, consider using std::format:
const std::string str = std::format("{}/{}.out.log", tmp_dir, test_case_name);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this idea but I'm afraid not all expected compilers support it

Copy link
Collaborator

@percona-ysorokin percona-ysorokin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@oleksandr-kachan oleksandr-kachan merged commit 7f4b4fd into percona:release-8.4.0-1 Jun 28, 2024
21 of 24 checks passed
@oleksandr-kachan oleksandr-kachan deleted the PS-9218-gcc-14 branch June 28, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants