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

replace sprintf with its safe version snprintf #5475

Closed
wants to merge 2 commits into from

Conversation

hliu18
Copy link
Contributor

@hliu18 hliu18 commented Jun 18, 2019

sprintf is unsafe and has buffer overrun risk. Replace it with the safer version snprintf where buffer size is supplied to avoid overrun.

merge from facebook/rocksdb
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@siying siying requested a review from sagar0 June 18, 2019 17:43
@siying
Copy link
Contributor

siying commented Jun 18, 2019

CC @zhichao-cao

Copy link
Contributor

@sagar0 sagar0 left a comment

Choose a reason for hiding this comment

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

The change to snprintf looks good to me. Just remove the unnecessary JeMalloc change in this PR.

thirdparty.inc Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@hliu18 has updated the pull request. Re-import the pull request

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@zhichao-cao
Copy link
Contributor

lgtm, thanks.

@facebook-github-bot
Copy link
Contributor

@sagar0 merged this pull request in 92f631d.

merryChris pushed a commit to merryChris/rocksdb that referenced this pull request Nov 18, 2019
Summary:
sprintf is unsafe and has buffer overrun risk. Replace it with the safer version snprintf where buffer size is supplied to avoid overrun.
Pull Request resolved: facebook#5475

Differential Revision: D15879481

Pulled By: sagar0

fbshipit-source-id: 7ae1958ffc9727fa50261dfbb98ddd74e70a72d8
facebook-github-bot pushed a commit that referenced this pull request Dec 12, 2022
Summary:
same motivations as #5475, applied to the last remaining `sprintf`.

Pull Request resolved: #11011

Reviewed By: pdillinger

Differential Revision: D41673500

Pulled By: ajkr

fbshipit-source-id: 88618ea791cafad86a9a491799c45979d46e3544
Qiaolin-Yu pushed a commit to asu-idi/rocksdb_block_cache_tracing that referenced this pull request Jan 30, 2023
Summary:
same motivations as facebook/rocksdb#5475, applied to the last remaining `sprintf`.

Pull Request resolved: facebook/rocksdb#11011

Reviewed By: pdillinger

Differential Revision: D41673500

Pulled By: ajkr

fbshipit-source-id: 88618ea791cafad86a9a491799c45979d46e3544
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants