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

memory leak in server.c monitor_add() / monitor_del() ? #1598

Open
michaelortmann opened this issue May 31, 2024 · 0 comments
Open

memory leak in server.c monitor_add() / monitor_del() ? #1598

michaelortmann opened this issue May 31, 2024 · 0 comments

Comments

@michaelortmann
Copy link
Member

michaelortmann commented May 31, 2024

Reviewing eggdrop for memory leaks i found suspicious code here:

I didnt check in real life, but looking at the source code, my understanding is:

monitor_add() allocates memory for a list member:

https://github.com/eggheads/eggdrop/blob/develop/src/mod/server.mod/server.c#L1244C12-L1245

but monitor_del() doesnt free that memory when it deletes a member from the list:

https://github.com/eggheads/eggdrop/blob/develop/src/mod/server.mod/server.c#L1284-L1292

if this is a real bug it could be related to or the same as #1545 (but unlikely coze this this bug would be older than eggdrop 1.9.2, see #1173 and #1545 (comment)))

please check if this is a real world memory leak.

a fix could look like (untested!):

/* Remove nickname from monitor list */
static int monitor_del (char *nick) {
  struct monitor_list *current = monitor;
  struct monitor_list *previous = NULL;

  while (current && rfc_casecmp(current->nick, nick)) {
    previous = current;
    current = current->next;
  }
  if (!current)
    return 1;
  if (current == monitor)
    monitor = current->next;
  else
    previous->next = current->next;
  nfree(current);

  dprintf(DP_SERVER, "MONITOR - %s\n", nick);
  return 0;
}

we have could also use the existing egg_list_delete(), but its not worth it here, because we would still have to search for nick and free the node. for eggdrop 2.0 we could change the mod API to provide a more generic egg_list, like egg_list_delete(list, cmp_func, free_func) that could handle all search and freeing needs. probably overkill ;)

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

No branches or pull requests

1 participant