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 762435 - memleaks in reverse/forward when calling _geocode_glib_cache_path_for_query()
memleaks in reverse/forward when calling _geocode_glib_cache_path_for_query()
Status: RESOLVED FIXED
Product: geocode-glib
Classification: Other
Component: general
3.19.x
Other Linux
: Normal normal
: ---
Assigned To: geocode-glib maintainer(s)
geocode-glib maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-02-22 11:01 UTC by Aleksander Morgado
Modified: 2016-02-22 18:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch. (1.16 KB, patch)
2016-02-22 11:02 UTC, Aleksander Morgado
accepted-commit_now Details | Review
Tester (1.25 KB, text/x-csrc)
2016-02-22 18:04 UTC, Aleksander Morgado
  Details

Description Aleksander Morgado 2016-02-22 11:01:50 UTC
The return of _geocode_glib_cache_path_for_query() isn't being properly freed in GeocodeReverse/GeocodeForward.

Patch following.
Comment 1 Aleksander Morgado 2016-02-22 11:02:37 UTC
Created attachment 321821 [details] [review]
Patch.
Comment 2 Bastien Nocera 2016-02-22 11:05:34 UTC
Review of attachment 321821 [details] [review]:

Looks good to me.
Comment 3 Bastien Nocera 2016-02-22 11:06:10 UTC
Review of attachment 321821 [details] [review]:

I would also add the output of valgrind showing the problem, for example.
Comment 4 Aleksander Morgado 2016-02-22 17:22:05 UTC
(In reply to Bastien Nocera from comment #3)
> Review of attachment 321821 [details] [review] [review]:
> 
> I would also add the output of valgrind showing the problem, for example.

It was actually just reading the code, didn't even run it under valgrind, but I can prepare a stupid test to showcase it.
Comment 5 Bastien Nocera 2016-02-22 17:55:44 UTC
(In reply to Aleksander Morgado from comment #4)
> (In reply to Bastien Nocera from comment #3)
> > Review of attachment 321821 [details] [review] [review] [review]:
> > 
> > I would also add the output of valgrind showing the problem, for example.
> 
> It was actually just reading the code, didn't even run it under valgrind,
> but I can prepare a stupid test to showcase it.

Then that'll do fine. Thanks for the patch!
Comment 6 Aleksander Morgado 2016-02-22 18:04:24 UTC
Created attachment 321880 [details]
Tester

Too late, I wrote a tester :)

        ==3365== 128 bytes in 1 blocks are definitely lost in loss record 2,226 of 2,357
        ==3365==    at 0x4C2AB5D: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
        ==3365==    by 0x566A3E7: g_realloc (in /usr/lib/libglib-2.0.so.0.4600.2)
        ==3365==    by 0x5685846: ??? (in /usr/lib/libglib-2.0.so.0.4600.2)
        ==3365==    by 0x5685B66: g_string_insert_len (in /usr/lib/libglib-2.0.so.0.4600.2)
        ==3365==    by 0x5650E77: ??? (in /usr/lib/libglib-2.0.so.0.4600.2)
        ==3365==    by 0x5652214: g_build_filename (in /usr/lib/libglib-2.0.so.0.4600.2)
        ==3365==    by 0x4E40AA0: _geocode_glib_cache_path_for_query (geocode-glib.c:92)
        ==3365==    by 0x4E4052F: geocode_reverse_resolve_async (geocode-reverse.c:454)
        ==3365==    by 0x400D55: main (in /home/aleksander/Development/clients/collabora/geocode-test)
Comment 7 Bastien Nocera 2016-02-22 18:05:53 UTC
Haha, thanks :)