Custom color fixes #187
Merged
Custom color fixes #187
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
November 4, 2024 20:07
In case you don't have custom_color defined in your config file (because you have old config file, for example), a default value is used. The default is specified as a string literal and its pointer is assigned to config->custom_color. The config_free() function calls g_free() on this pointer uncoditionally but you can't do that on a pointer to string literal so this causes a segfault. To avoid that, we can g_strdup() the string literal, just like it is done for example with config->text_font.
In case "auto_save" is not set in the config file, the error pointer will point to a GError that is printed by the g_info() but the pointer is not freed after using. This causes not only memory leak but also a warning from Glib in case you also do not have "custom_color" in your config as g_key_file_get_string() will get a pointer that already points to GError. The warning message looks like this: (swappy:492252): GLib-WARNING **: 20:11:27.212: GError set over the top of a previous GError or uninitialized memory. This indicates a bug in someone's code. You must ensure an error is NULL before it's set. The overwriting error message was: Key file does not have key “custom_color” in group “Default” A solution is to properly free the error pointer after it is no longer needed and before it is reused.
|
Thanks, I squashed into 1 commit, this is a |
|
Sorry, I didn't fully understand your commit message convention. I'm used to the way Linux kernel does it, for example, where we just point to the subsystem the commit is related to. My way of thinking was that this is a fix related to a feat(config). Now I get the convention and it indeed makes more sense. I can also see I screwed up with my gitconfig filtering on a new system and wrong email address was used for a commit. It was supposed to be my private, not company's email :) In any case, thank you for merging this so quickly. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When trying to rebase my changes on top of the latest master branch, I found out two problems that slipped through the review of PR#185. Fortunately the fixes are rather straightforward.