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 mem leak in conf_string_list_add #1932

Merged

Conversation

H3rnand3zzz
Copy link
Contributor

See commit message.

@jubalh
Copy link
Member

jubalh commented Nov 15, 2023

I checked the MUC and saw that @sjaeckel diff looked like:

       ```
       diff --git a/src/config/conflists.c b/src/config/conflists.c
       index 4032878e..838b611e 100644
       --- a/src/config/conflists.c
       +++ b/src/config/conflists.c
       @@ -51,3 +51,3 @@ conf_string_list_add(GKeyFile* keyfile, const char* const group, const char* con
            gsize length;
       -    auto_gcharv gchar** list = g_key_file_get_string_list(keyfile, group, key, &length, NULL);
       +    gchar** list = g_key_file_get_string_list(keyfile, group, key, &length, NULL);

       @@ -62,2 +62,3 @@ conf_string_list_add(GKeyFile* keyfile, const char* const group, const char* con
                if (strcmp(list[i], item) == 0) {
       +            g_strfreev(list);
                    return FALSE;
       @@ -69,3 +70,3 @@ conf_string_list_add(GKeyFile* keyfile, const char* const group, const char* con
            for (gsize i = 0; i < length; ++i) {  
       -        new_list[i] = g_strdup(list[i]);
       +        new_list[i] = list[i];
            }
       @@ -73,2 +74,3 @@ conf_string_list_add(GKeyFile* keyfile, const char* const group, const char* con
            new_list[length + 1] = NULL;
       +    g_free(list);

       ```

@jubalh jubalh requested a review from sjaeckel November 15, 2023 18:18
@jubalh jubalh added the cleanup label Nov 15, 2023
@jubalh jubalh added this to the next milestone Nov 15, 2023
Introduced in 1cf7973
Issue reported and fix proposed by @sjaeckel
@H3rnand3zzz H3rnand3zzz force-pushed the cleanup/refactor-conf-string-list branch from 9dbf6f4 to bd2821f Compare November 17, 2023 20:15
@H3rnand3zzz
Copy link
Contributor Author

The solution has been improved.

@sjaeckel
Copy link
Member

The solution has been improved.

Indeed, looks very good now!

@jubalh jubalh merged commit e143513 into profanity-im:master Nov 20, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants