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 669334 - fix memory leak in bookmark file parser
fix memory leak in bookmark file parser
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
2.31.x
Other All
: Normal minor
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2012-02-03 18:40 UTC by Ravi Sankar Guntur
Modified: 2012-02-04 15:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix a memory leak (1016 bytes, patch)
2012-02-03 18:46 UTC, Ravi Sankar Guntur
accepted-commit_now Details | Review
single exit with free (2.55 KB, patch)
2012-02-04 13:12 UTC, Ravi Sankar Guntur
committed Details | Review

Description Ravi Sankar Guntur 2012-02-03 18:40:47 UTC
fix memory leak in g_bookmark_file_parse().

==605== 1,315 (104 direct, 1,211 indirect) bytes in 1 blocks are definitely lost in loss record 111 of 119
==605==    at 0x4028876: malloc (vg_replace_malloc.c:236)
==605==    by 0x407B62A: standard_malloc (gmem.c:85)
==605==    by 0x407B9D2: g_malloc (gmem.c:159)
==605==    by 0x4078F27: g_markup_parse_context_new (gmarkup.c:228)
==605==    by 0x404BC1F: g_bookmark_file_load_from_data (gbookmarkfile.c:1441)
==605==    by 0x404BDDB: g_bookmark_file_load_from_file (gbookmarkfile.c:1735)
==605==    by 0xBED67E9F: ???
==605== 
==605== 19,682 (1,664 direct, 18,018 indirect) bytes in 16 blocks are definitely lost in loss record 119 of 119
==605==    at 0x4028876: malloc (vg_replace_malloc.c:236)
==605==    by 0x407B62A: standard_malloc (gmem.c:85)
==605==    by 0x407B9D2: g_malloc (gmem.c:159)
==605==    by 0x4078F27: g_markup_parse_context_new (gmarkup.c:228)
==605==    by 0x404BC1F: g_bookmark_file_load_from_data (gbookmarkfile.c:1441)
==605==    by 0x404BDDB: g_bookmark_file_load_from_file (gbookmarkfile.c:1735)
==605==    by 0x4065C4F: g_str_equal (ghash.c:1697)
==605==    by 0x4065C1F: ??? (ghash.c:1668)
Comment 1 Ravi Sankar Guntur 2012-02-03 18:46:35 UTC
Created attachment 206708 [details] [review]
fix a memory leak

please review.
Comment 2 Colin Walters 2012-02-03 19:06:23 UTC
Review of attachment 206708 [details] [review]:

Looks correct, though you could also change this function to single-exit-and-free style (since it would be trivial to do so) while you have the patient open.

Do you have GNOME commit access?
Comment 3 Ravi Sankar Guntur 2012-02-04 04:12:15 UTC
thank you. would consider your suggestion.

nope, don't have commit access.
Comment 4 Ravi Sankar Guntur 2012-02-04 13:12:48 UTC
Created attachment 206755 [details] [review]
single exit with free

please review.
Comment 5 Colin Walters 2012-02-04 15:09:45 UTC
Review of attachment 206755 [details] [review]:

::: glib/gbookmarkfile.c
@@ +1460,3 @@
+   }
+ 
+  if (context)

This if (context) looks unnecessary.  I've gone ahead and removed it, and pushed your patch.  Thanks!

Now that I look at this code more closely though, I am confused why we are allocating separate "parse_error" and "end_error".   Emmanuele?