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

Engine: use safe strcpy function instead of snprintf where truncation expected #2569

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

Conversation

ivan-mogilko
Copy link
Contributor

@ivan-mogilko ivan-mogilko commented Nov 7, 2024

Fix #2567

This replaces uses of snprintf in the code where truncation of the source string is intended.
Added a "ags_strncpy_s", using "strncpy_s" from the C11 extension as a reference (see https://en.cppreference.com/w/c/string/byte/strncpy)

@ivan-mogilko ivan-mogilko added the context: code fixing/improving existing code: refactor, optimise, tidy... label Nov 7, 2024
@ivan-mogilko ivan-mogilko force-pushed the fix--nonstd-snprintf-truncation branch from 14b796d to 8ab4bc4 Compare November 7, 2024 00:23
@ivan-mogilko ivan-mogilko force-pushed the fix--nonstd-snprintf-truncation branch 2 times, most recently from a62820e to b7fdfe4 Compare November 7, 2024 01:00
@ivan-mogilko ivan-mogilko changed the title Engine: use ags_strncpy_s instead of snprintf where truncation expected Engine: use CStrCpy instead of snprintf where truncation expected Nov 7, 2024
@ivan-mogilko ivan-mogilko force-pushed the fix--nonstd-snprintf-truncation branch from b7fdfe4 to 77cfaf4 Compare November 7, 2024 16:44
@ivan-mogilko ivan-mogilko changed the title Engine: use CStrCpy instead of snprintf where truncation expected Engine: use safe strcpy function instead of snprintf where truncation expected Nov 7, 2024
@ivan-mogilko
Copy link
Contributor Author

@ericoporto i copied error values and some parts from your ags_strncpy_s implementation in ags4, but copy characters in a for loop in place instead of calling snprintf, because extra call to a formatting function seems unnecessary in this case.

Also added asserts to help catch errors.

@ivan-mogilko ivan-mogilko force-pushed the fix--nonstd-snprintf-truncation branch 2 times, most recently from 403f409 to 4c0bbcb Compare November 7, 2024 18:51
@ericoporto
Copy link
Member

Looks fine, there is a change to Engine/main/update.cpp I can’t tell if it was intended or not.

@ivan-mogilko ivan-mogilko force-pushed the fix--nonstd-snprintf-truncation branch from 4c0bbcb to 2f4345e Compare November 7, 2024 20:33
@ivan-mogilko
Copy link
Contributor Author

Looks fine, there is a change to Engine/main/update.cpp I can’t tell if it was intended or not.

Oops, that was a fix for another warning, it's already in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context: code fixing/improving existing code: refactor, optimise, tidy...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Truncation warning in Common/game/main_game_file.cpp
2 participants