server icon indicating copy to clipboard operation
server copied to clipboard
verified publisher icon MariaDB

Convert instances of `sprintf` in `storage/` to `snprintf`

Open tgross35 opened this issue 1 year ago • 4 comments

Remove use of sprintf in favor of snprintf where indicated by deprecation warnings.

  • [x] The Jira issue number for this PR is: MDEV-______

Description

This is a portion of #2958

Release Notes

None needed

How can this PR be tested?

Compile with -Wdeprecated-declarations (I believe it is enabled by default for the relevant files) and use a version of libc that marks sprintf deprecated.

Basing the PR against the correct MariaDB version

  • [x] This is a new feature and the PR is based against the latest MariaDB development 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

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

tgross35 avatar Feb 20 '24 07:02 tgross35

@LinuxJedi, @vuvova, as @ottok and I and others at @aws have been saying for a while: it's extremely difficult for external contributors to evaluate the readiness of their PRs when there are large numbers of ongoing CI failures on the upstream branches.

This PR is a case in point.

The builds currently failing on this PR… image

… are the same as the builds failing on the upstream parent commit (plus one extra) (https://github.com/MariaDB/server/commits/a6833faf119e287d63dd367341607846505f1dbc): image

dlenski avatar Feb 20 '24 20:02 dlenski

Yes, I know. Making all tests pass is still work in progress

vuvova avatar Feb 20 '24 20:02 vuvova

@vuvova wrote:

Yes, I know. Making all tests pass is still work in progress

Consider disabling the failing ones altogether until you can stabilize them, so as not to confuse and frustrate new external contributors.

dlenski avatar Feb 20 '24 21:02 dlenski

The CI does say which tests are required and which aren't, I split this up because required tests were failing and because I can't actually build on my local machine with only a portion of these patches - which is why I started these in the first place https://mariadb.zulipchat.com/#narrow/stream/118759-general/topic/Building.20on.20aarch64.20macos. But yes, it would be nice to see an entire green pipeline :)

Thank you for the thorough review, I'll update in a bit.

tgross35 avatar Feb 20 '24 21:02 tgross35