Skip to content

Fix tdns leak and init attr only once - #1838

Merged
vanosg merged 4 commits into
eggheads:developfrom
michaelortmann:pthread_mutex_destroy
May 10, 2026
Merged

vanosg merged 4 commits into
eggheads:developfrom
michaelortmann:pthread_mutex_destroy

Conversation

@michaelortmann

Copy link
Copy Markdown
Member

Found by: https://github.com/michaelortmann
Patch by: https://github.com/michaelortmann
Fixes:

One-line summary:
Fix tdns leak and init attr only once

Additional description (if needed):
Also cleaned up code structure, moved tdns init code from main.c to dns.c:init_tdns()

Test cases demonstrating functionality (if applicable):
before:

$ valgrind --leak-check=full ./eggdrop -t BotA.conf 2>&1|grep dns.c
.tcl dnslookup google.com foo
.die
==11075==    by 0x24E202: core_dns_ipbyhost (dns.c:620)
==11075==    by 0x24E973: tcl_dnsipbyhost (dns.c:369)
==11075==    by 0x24E640: tcl_dnslookup (dns.c:739)

if (pthread_attr_init(&attr)) {

after:

$ valgrind --leak-check=full ./eggdrop -t BotA.conf 2>&1|grep dns.c
.tcl dnslookup google.com foo
.die

@vanosg vanosg added this to the v1.10.2 milestone Nov 30, 2025

@thommey thommey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The pthread_mutex_destroy() you add to core_dns_ipbyhost() should probably also be added to core_dns_hostbyip(), right?

@michaelortmann

Copy link
Copy Markdown
Member Author

The pthread_mutex_destroy() you add to core_dns_ipbyhost() should probably also be added to core_dns_hostbyip(), right?

yes. so i added them.

ready for review again.

@thommey thommey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@vanosg
vanosg merged commit 0eee57c into eggheads:develop May 10, 2026
26 checks passed
@michaelortmann
michaelortmann deleted the pthread_mutex_destroy branch May 10, 2026 18:06
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.

3 participants