Skip to content

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