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 various memory leaks #1357

Closed
wants to merge 1 commit into from
Closed

Conversation

yukiteruamano
Copy link
Contributor

Hi, this changes fix a few memory leaks detected by clang-static-analyzer. The fixes are in backend/egl.c, backend/glx.c, src/config.c and transition/script.c

There are other issues detected, but touching them will break picom, so we'll have to keep checking these new fixes. At the moment, these fixes work perfectly on my machine (FreeBSD) for both amd64 and i386.

As I fix the rest of the bugs detected with clang-static-analyzer, I'll upload the rest of the commits.

Comment on lines -1001 to 1003
free(info);
}
free(info);
}
Copy link
Owner

Choose a reason for hiding this comment

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

this looks wrong, won't this leak info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I check again, but in its original position where the fault appeared.

Copy link
Owner

Choose a reason for hiding this comment

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

ok, but can you explain why this leaks? or are you making changes without understanding just to make warnings go away?

@@ -609,6 +612,7 @@ void *parse_window_shader_prefix(const char *src, const char **end, void *user_d
char *tmp = (char *)trim_both(untrimed_shader_source, &length);
tmp[length] = '\0';
char *shader_source = NULL;
free(untrimed_shader_source);
Copy link
Owner

Choose a reason for hiding this comment

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

this is unnecessary. untrimed_shader_source is automatically freed when it's out of scope.

@@ -581,6 +583,7 @@ void parse_debug_options(struct debug_options *debug_options) {
parse_debug_option_single(needle, debug_options);
needle = strtok_r(NULL, ";", &tmp);
}
free(debug_copy);
Copy link
Owner

Choose a reason for hiding this comment

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

unnecessary, same as untrimed_shader_source

Comment on lines +490 to +494
free(config_home);
return ret;
}
}
free(config_home);
Copy link
Owner

Choose a reason for hiding this comment

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

same as untrimed_shader_source

@yshui
Copy link
Owner

yshui commented Oct 11, 2024

hey, thanks for investigating this!

if you look at the definition of scoped_charp, they use __attribute__((cleanup)) to make sure they are freed automatically. other than those this looks fine 👍

Copy link
Owner

@yshui yshui left a comment

Choose a reason for hiding this comment

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

pls fix

@yukiteruamano
Copy link
Contributor Author

pls fix

I'm reworking the commit, this time taking into account what you've commented, thanks for the feedback.

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