Skip to content

convert sprintf calls to snprintf #267

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

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

Conversation

lunarpapillo
Copy link

Partly addresses:

mac build provokes warnings
https://github.com/ValveSoftware/Fossilize/issues/266

Many implementations have deprecated sprintf() in favor of snprintf(), including macOS with latest Xcode and Windows if _CRT_SECURE_NO_WARNINGS is set.

This change converts all uses of sprintf() to snprintf(). In most cases this is simply converting
sprintf(x, ...)
to
snprintf(x, sizeof(x), ...)

But there are a few more complicated constructions in fossilize_replay.cpp, fossilize_db.cpp, and layer/instance.cpp.

Partly addresses:

    mac build provokes warnings
    ValveSoftware#266

Many implementations have deprecated sprintf() in favor of snprintf(), including
macOS with latest Xcode and Windows if _CRT_SECURE_NO_WARNINGS is set.

This change converts all uses of sprintf() to snprintf().  In most cases this is
simply converting
    sprintf(x, ...)
to
    snprintf(x, sizeof(x), ...)

But there are a few more complicated constructions in fossilize_replay.cpp,
fossilize_db.cpp, and layer/instance.cpp.
@HansKristian-Work
Copy link
Collaborator

Does this actually unblock something, or is it purely to avoid warnings?

@spencer-lunarg
Copy link

spencer-lunarg commented Jun 2, 2025

I am 99% this is purely a warning and just reporting to "fix it now before it causes issues down the road"

@lunarpapillo
Copy link
Author

I am 99% this is purely a warning and just reporting to "fix it now before it causes issues down the road"

I'm 100% sure. :-)

Nothing is blocked (though we did have to alter the SDK build for this repository to turn off "warnings-are-errors" in order to unblock).

The warning that sprintf() is deprecated was stronger on some compilers than others, suggesting that support for the function will eventually be removed. I'm not sure whether that's something to plan for, or a toothless threat.

I would not be concerned if smart people like y'all decided this wasn't ever going to be a real issue, and decided to close the PR without merge.

(I only made a PR because I'd gone through the effort to analyze the warnings after they first broke the SDK build, and I thought I understood at that point how to fix them, and thought it would be a shame to just drop them.)

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.

3 participants