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

Use length-bounded string/memory functions #7709

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

rubiefawn
Copy link
Contributor

@rubiefawn rubiefawn commented Feb 14, 2025

Resolves #3949 by replacing all calls to sprintf() and strcpy() in first-party code (read: not in a git submodule or src/3rdparty) with calls to snprintf() or some other reasonable alternative, depending on context.

This does not resolve the memory management issue raised in this comment.

This replaces all calls to sprintf() in first-party code with calls to
snprintf() or some other reasonable alternative.
Remove bitmask, since the format specifier already restricts the output
to 2 characters per byte
I used a different name in my test code to make sure my method was
working properly and forgot to change the name back. Fixed!
@sakertooth
Copy link
Contributor

I'd be reluctant to change anything in MidiImport/portsmf. This seems to be a third party library that wasn't pulled into as a Git submodule for some reason.

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

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

Looks good, just had a few thoughts/questions.

@rubiefawn rubiefawn changed the title Prefer snprintf() over sprintf() Use length-bounded string/memory functions Mar 7, 2025
len could be < size
This is done by a) explicitly initializing buffers to 0, and b) providing snprintf() with one less than the buffer size, so that the last remaining element will always retain its initial value (which is 0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audit/replace use of sprintf() and friends...
3 participants