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

Add missing LEX_STRING::strs for my_snprintf #3538

Open
wants to merge 1 commit into
base: 11.6
Choose a base branch
from

Conversation

ParadoxV5
Copy link
Contributor

  • The Jira issue number for this PR is: MDEV-21978

Description

This commit backports a fix group from #3360 for 11.5.
These are the only mistakes introduced during 11.5.x.

What problem is the patch trying to solve?

When these members switched from plain char* to LEX_CSTRING, not all usages were converted. Specifically, in this commit are args of my_snprintf derivatives.
Because until MDEV-21978 (#3360), automated type checks were unavailable for those functions due to their incompatibility, so these tools didn’t catch them.

If some output changed that is not visible in a test case, what was it looking like before the change and how it's looking with this patch applied?

str is the initial member of LEX_CSTRING (typedef of struct st_mysql_const_lex_string), so this difference did not cause incompatibility on most platforms with seamless alignments;
howëver, my co-mentor notes that Visual Studio reads garbage.

Release Notes

How can this PR be tested?

This difference only manifests when LEX_CSTRING::str does not align on the 0th index of the overall LEX_CSTRING.
Besides using Visual Studio as noted, another deception to disregard this leniency is to shift the str member away from the 0th alignment, such as adding a padding member before it or moving it after member length.

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@ParadoxV5
Copy link
Contributor Author

Oh great, the sporadic failures came back to haunt us.

@grooverdan
Copy link
Member

Looks all right. Thanks. 11.5 is now EOL - can you rebase to 11.6 please.

@ParadoxV5
Copy link
Contributor Author

👀 Oh right, 11.5 is a rolling release and has been superceded by 11.6.
The Maintenance Policy links to the rolling release system announcement but doesn’t (explicitly) say much itself.

@ParadoxV5 ParadoxV5 changed the base branch from 11.5 to 11.6 October 3, 2024 03:12
When these members switched from plain `char*` to `LEX_CSTRING`,
not all usages were converted. Specifically, in this commit are args of
`my_snprintf` derivatives. Because until MDEV-21978,
automated type checks were unavailable for those functions due to
their incompatibility, so these tools didn’t catch them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants