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

(WIP) Fix optimize-kicks == 2 #1490

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

michaelortmann
Copy link
Member

@michaelortmann michaelortmann commented Oct 16, 2023

Found by: michaelortmann
Patch by: michaelortmann
Fixes:

One-line summary:
Fix optimize-kicks == 2

Additional description (if needed):

  • Fix snprintf() overlaps destination object for optimize-kicks == 2
    This problem was hidden because the code used egg_snprintf(), which gcc cant check and throw the following warning properly:
gcc -fPIC -g -O2 -pipe -Wall -I. -I../../.. -I../../.. -I../../../src/mod  -DHAVE_CONFIG_H -I/usr/include -g3 -DDEBUG -DDEBUG_ASSERT -DDEBUG_MEM -DDEBUG_DNS  -DMAKING_MODS -c .././server.mod/server.c && mv -f server.o ../
.././server.mod/server.c: In function ‘parse_q’:
[...]
  592 |             snprintf(newnicks, sizeof newnicks, "%s,%s", newnicks, newnick);
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.././server.mod/server.c:592:13: warning: ‘snprintf’ argument 4 overlaps destination object ‘newnicks’ [-Wrestrict]
.././server.mod/server.c:571:54: note: destination object referenced by ‘restrict’-qualified argument 1 was declared here
  571 |   char buf[SENDLINEMAX], *msg, *nicks, *nick, *chan, newnicks[SENDLINEMAX],
      |                                                      ^~~~~~~~
  • egg_snprintf() -> snprintf()
  • Fix a format truncation warning by properly adjusting size of newnicks[]
  • The rest of the code compares the message in the queue with "KICK" (4 chars) and not "KICK " (5 chars), which is sufficient.
    lll
  • Do snprintf(newmsg) only if we have to and no need to strlen() when we already got the len returned from snprintf()

Test cases demonstrating functionality (if applicable):
Is there a logic error in

egg_snprintf(newnicks, sizeof newnicks, ",%s", nick);
? No concatenation?
Still untested. Please can someone step in and test this PR?

@michaelortmann michaelortmann changed the title Fix snprintf() overlaps destination object for optimize_kicks == 2 Fix optimize-kicks == 2 Oct 16, 2023
@michaelortmann michaelortmann changed the title Fix optimize-kicks == 2 (Untested) Fix optimize-kicks == 2 Oct 16, 2023
@vanosg
Copy link
Member

vanosg commented Dec 30, 2023

Please test and paste results, or close. Thanks!

@michaelortmann michaelortmann changed the title (Untested) Fix optimize-kicks == 2 (WIP) Fix optimize-kicks == 2 Jun 15, 2024
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