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

Fix truncation / buffer sizes #1034

Merged
merged 2 commits into from
Dec 19, 2020
Merged

Conversation

michaelortmann
Copy link
Member

@michaelortmann michaelortmann commented Oct 5, 2020

Found by: michaelortmann
Patch by: michaelortmann
Fixes:

One-line summary:
Fix truncation / buffer sizes

Additional description (if needed):
Found with #1028, but can/should be merged independently before #1028.

Test cases demonstrating functionality (if applicable):
it should be possible to demonstrate a truncated notice in

egg_snprintf(s1, sizeof s1, "NOTICE %s :[%s] %s: %s.", nick, u2->handle,
and to demonstrate that this PR will fix this. But it should be enough to know, that this PR will fix compiler warnings that would occur after #1028.

Copy link
Member

@vanosg vanosg left a comment

Choose a reason for hiding this comment

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

good catches! can you double-check these values and adjust if necessary?

src/mod/irc.mod/msgcmds.c Show resolved Hide resolved
src/mod/irc.mod/msgcmds.c Show resolved Hide resolved
@michaelortmann
Copy link
Member Author

michaelortmann commented Nov 3, 2020

Geo: For the uhostlen mess see also #650. Many compiler warnings are currently shaddowed by using egg_snprintf() instead snprintf(). For that see also #1028. There is more to come, but i want to see this PR merged rather soon instead of making one big PR, because not all compiler warnings about buffer sizes / truncation are equal. This here is one of the more serious ones and not only cosmetic.

@vanosg vanosg merged commit 9784ccb into eggheads:develop Dec 19, 2020
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.

2 participants