After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 677720 - [patch] bunch of memory leaks
[patch] bunch of memory leaks
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-06-08 18:23 UTC by Pavel Vasin
Modified: 2012-06-12 09:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-ephy-profile-utils-fix-memory-leak.patch (736 bytes, patch)
2012-06-08 18:24 UTC, Pavel Vasin
committed Details | Review
0002-ephy-bookmark-action-fix-memory-leak.patch (752 bytes, patch)
2012-06-08 18:25 UTC, Pavel Vasin
committed Details | Review
0003-ephy-bookmarks-fix-memory-leaks.patch (1.32 KB, patch)
2012-06-08 18:25 UTC, Pavel Vasin
committed Details | Review
0004-ephy-web-view-fix-GList-leak.patch (1.30 KB, patch)
2012-06-08 18:26 UTC, Pavel Vasin
committed Details | Review
0005-ephy-session-fix-memory-leak-in-ephy_session_save.patch (1.87 KB, patch)
2012-06-08 18:26 UTC, Pavel Vasin
needs-work Details | Review
0006-ephy-completion-model-fix-GList-of-EphyHistoryURL-le.patch (790 bytes, patch)
2012-06-08 18:27 UTC, Pavel Vasin
committed Details | Review
0007-ephy-bookmarks-editor-fix-memory-leak.patch (1.63 KB, patch)
2012-06-11 10:54 UTC, Pavel Vasin
committed Details | Review
0008-ephy-bookmarks-editor-fix-GList-leaks.patch (1.24 KB, patch)
2012-06-11 10:55 UTC, Pavel Vasin
committed Details | Review
0005-ephy-session-fix-memory-leak-in-write_tab.patch (1.22 KB, patch)
2012-06-11 16:49 UTC, Pavel Vasin
committed Details | Review

Description Pavel Vasin 2012-06-08 18:23:35 UTC
Let's fix them
Comment 1 Pavel Vasin 2012-06-08 18:24:27 UTC
Created attachment 215979 [details] [review]
0001-ephy-profile-utils-fix-memory-leak.patch
Comment 2 Pavel Vasin 2012-06-08 18:25:10 UTC
Created attachment 215980 [details] [review]
0002-ephy-bookmark-action-fix-memory-leak.patch
Comment 3 Pavel Vasin 2012-06-08 18:25:44 UTC
Created attachment 215981 [details] [review]
0003-ephy-bookmarks-fix-memory-leaks.patch
Comment 4 Pavel Vasin 2012-06-08 18:26:19 UTC
Created attachment 215982 [details] [review]
0004-ephy-web-view-fix-GList-leak.patch
Comment 5 Pavel Vasin 2012-06-08 18:26:57 UTC
Created attachment 215983 [details] [review]
0005-ephy-session-fix-memory-leak-in-ephy_session_save.patch
Comment 6 Pavel Vasin 2012-06-08 18:27:37 UTC
Created attachment 215984 [details] [review]
0006-ephy-completion-model-fix-GList-of-EphyHistoryURL-le.patch
Comment 7 Pavel Vasin 2012-06-11 10:54:26 UTC
Created attachment 216099 [details] [review]
0007-ephy-bookmarks-editor-fix-memory-leak.patch
Comment 8 Pavel Vasin 2012-06-11 10:55:23 UTC
Created attachment 216100 [details] [review]
0008-ephy-bookmarks-editor-fix-GList-leaks.patch
Comment 9 Claudio Saavedra 2012-06-11 14:02:29 UTC
Review of attachment 215979 [details] [review]:

Good catch.
Comment 10 Claudio Saavedra 2012-06-11 14:04:14 UTC
Review of attachment 215980 [details] [review]:

LRTM
Comment 11 Claudio Saavedra 2012-06-11 14:05:47 UTC
Review of attachment 215981 [details] [review]:

Maybe you can merge it with the previous one, but otherwise looks right.
Comment 12 Claudio Saavedra 2012-06-11 14:08:46 UTC
Review of attachment 215982 [details] [review]:

Looks about right, too.
Comment 13 Claudio Saavedra 2012-06-11 14:16:16 UTC
Review of attachment 215983 [details] [review]:

::: src/ephy-session.c
@@ -650,1 @@
 	   EphyEmbed *embed)

This is actually write_tab(), not ephy_session_save(), the comment is wrong, can you review that please?

@@ +665,3 @@
 	}
 	ret = xmlTextWriterWriteAttribute (writer, (xmlChar *) "url",
+					   (const xmlChar *) (fake_address ? fake_address : address));

Why not just freeing fake_address right here rather than at the end?
Comment 14 Claudio Saavedra 2012-06-11 14:22:29 UTC
Review of attachment 215984 [details] [review]:

Seems about right!
Comment 15 Claudio Saavedra 2012-06-11 15:25:46 UTC
Review of attachment 216099 [details] [review]:

::: src/bookmarks/ephy-bookmarks-editor.c
@@ +1473,3 @@
+    return result;
+}
+

To me this really looks like a deficience in the WK IconDatabase API, since there's no way to find out if a favicon exists for an url without allocating  a string. I think I'll file a bug in wk about this. Meanwhile I think this patch will do.
Comment 16 Claudio Saavedra 2012-06-11 15:28:06 UTC
Review of attachment 216100 [details] [review]:

Another good catch.
Comment 17 Claudio Saavedra 2012-06-11 15:28:25 UTC
Review of attachment 216100 [details] [review]:

Another good catch.
Comment 18 Pavel Vasin 2012-06-11 16:41:33 UTC
(In reply to comment #10)
> LRTM

What does this mean?
Comment 19 Pavel Vasin 2012-06-11 16:49:54 UTC
Created attachment 216134 [details] [review]
0005-ephy-session-fix-memory-leak-in-write_tab.patch

> the comment is wrong

Fixed

> Why not just freeing fake_address right here rather than at the end?

Thanks, I missed this.
Comment 20 Claudio Saavedra 2012-06-11 19:13:05 UTC
(In reply to comment #18)
> (In reply to comment #10)
> > LRTM
> 

Looks right to me :)

Do you have a git account or should I push them for you?
Comment 21 Pavel Vasin 2012-06-12 06:54:05 UTC
(In reply to comment #20)
> Do you have a git account or should I push them for you?

I don't have git account, please commit.
Comment 22 Claudio Saavedra 2012-06-12 09:32:13 UTC
I changed the variable name in the last patch, fake_address didn't sound very
good. Other than that, I pushed all the patches. Thank you!