MDEV-35494 fil_space_t::fil_space_t() may be unsafe with GCC -flifetime-dse #3659
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
|
|
dr-m
force-pushed
the
10.6-MDEV-35494
branch
2 times, most recently
from
6458cf3 to
9399900
Compare
There was a problem hiding this comment.
@dr-m The initialization of fil_space_t and other refactoring looks good to me. I have some minor comments/suggestion on the patch. Please have a look.
mariadb-DebarunBanerjee
approved these changes
Dec 3, 2024
There was a problem hiding this comment.
Thanks @dr-m for addressing the comments. Looks good to me. I have one observation in the latest change.
…me-dse fil_space_t::create(): Instead of invoking the default fil_space_t constructor on a zero-filled buffer, allocate an uninitialized buffer and invoke an explicitly defined constructor on it. Also, specify initializer expressions for all constant data members, so that all of them will be initialized in the constructor. fil_space_t::being_imported: Replaces part of fil_space_t::purpose. fil_space_t::is_being_imported(), fil_space_t::is_temporary(): Replaces fil_space_t::purpose. fil_space_t::id: Changed the type from ulint to uint32_t to reduce incompatibility with later branches that include commit ca501ff (MDEV-26195). fil_space_t::try_to_close(): Do not attempt to close files that are in an I/O bound phase of ALTER TABLE…IMPORT TABLESPACE. log_file_op, first_page_init: recv_spaces_t: Use uint32_t for the tablespace id. Reviewed by: Debarun Banerjee
dr-m
force-pushed
the
10.6-MDEV-35494
branch
from
1a4b97f to
bfe7c8f
Compare
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.
Description
fil_space_t::create(): Instead of invoking the defaultfil_space_tconstructor on a zero-filled buffer, allocate an uninitialized buffer and invoke an explicitly defined constructor on it. Also, specify initializer expressions for all constant data members, so that all of them will be initialized in the constructor.fil_space_t::being_imported: Replaces part offil_space_t::purpose.fil_space_t::is_being_imported(),fil_space_t::is_temporary(): Replacesfil_space_t::purpose.fil_space_t::id: Changed the type fromulinttouint32_tto reduce incompatibility with later branches that includecommit ca501ff (MDEV-26195).
log_file_op,first_page_init:recv_spaces_t: Useuint32_tfor the tablespace id.Release Notes
This change should not have any noticeable effect.
How can this PR be tested?
This low level code is rather well covered by the regression test suite. As always, some additional stress testing with the random query generator (RQG) will be useful.
Basing the PR against the correct MariaDB version
mainbranch.This is a portability bug fix that would be partly applicable to 10.5, but the 10.5 version would require some nontrivial adjustments, and the 10.5 branch is close to its end of life (EOL).
PR quality check