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.

Sign up for GitHub

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

Jump to bottom

Use snprintf instead of sprintf in implot_demo.cpp #426

Merged
merged 2 commits into from
Nov 25, 2022
Merged

Use snprintf instead of sprintf in implot_demo.cpp #426

merged 2 commits into from
Nov 25, 2022

Conversation

jminor
Copy link
Contributor

@jminor jminor commented Nov 24, 2022

sprintf is marked as deprecated and/or unsafe by some compilers. For example, clang complains:

implot_demo.cpp:840:17: warning: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Wdeprecated-declarations]

This PR replaces all calls to sprintf in implot_demo.cpp with snprintf to address this concern.

Addresses #421

BenBE
BenBE approved these changes Nov 24, 2022
View reviewed changes
Copy link
Contributor

@BenBE BenBE left a comment

Choose a reason for hiding this comment

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

LGTM. And the length for label is also sufficient (would otherwise potentially throw compile warnings depending on -Wformat=??? setting).

BenBE
BenBE reviewed Nov 24, 2022
View reviewed changes
implot_demo.cpp Outdated Show resolved Hide resolved
@epezent epezent merged commit fcb51d2 into epezent:master Nov 25, 2022
@jminor jminor deleted the snprintf branch November 25, 2022 18:24
Ben1138 pushed a commit to Ben1138/implot that referenced this pull request Oct 2, 2024
@jminor @BenBE
Use snprintf instead of sprintf in implot_demo.cpp (epezent#426)
38b9466 
* Use snprintf instead of sprintf in implot_demo.cpp

* Enlarged text buffer

Co-authored-by: BenBE <[email protected]>

Co-authored-by: BenBE <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenBE BenBE BenBE approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jminor @BenBE @epezent