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 757221 - Memory leak in gtk-3.0.m4
Memory leak in gtk-3.0.m4
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
3.18.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-10-27 21:04 UTC by Dhiru Kholia
Modified: 2015-11-03 12:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to fix the memory leak (1.59 KB, patch)
2015-10-27 21:13 UTC, Dhiru Kholia
committed Details | Review

Description Dhiru Kholia 2015-10-27 21:04:52 UTC
gtk-3.0.m4 file does "tmp_version = g_strdup("$min_gtk_version");" but is does *not* do "g_free(tmp_version);".

This results in a memory leak. This memory leak is causing Wireshark builds to fail during the "./configure --enable-asan" phase.

See https://code.wireshark.org/review/11283 for details.

A simple fix would be to add a line saying "g_free(tmp_version;" to the gtk-3.0.m4 file.

A better fix would be to remove this "HP/UX 9 hack", something like https://git.gnome.org/browse/glib/diff/m4macros/glib-2.0.m4?id=7f5b2901cf5bea290c11133dad16850176178dad commit.
Comment 1 Dhiru Kholia 2015-10-27 21:13:42 UTC
Created attachment 314259 [details] [review]
patch to fix the memory leak
Comment 2 Matthias Clasen 2015-10-28 03:06:22 UTC
it is really not our fault if wireshark thinks it is a good idea to check configure test programs for memory leaks and fail as a result...
Comment 3 Dhiru Kholia 2015-10-29 09:34:47 UTC
Wouldn't it be super nice to fix the memory leak regardless? Why have it when it can be gone easily enough?
Comment 4 Emmanuele Bassi (:ebassi) 2015-10-29 09:43:27 UTC
(In reply to Dhiru Kholia from comment #3)
> Wouldn't it be super nice to fix the memory leak regardless? Why have it
> when it can be gone easily enough?

Because it's not a leak: it's a one-off allocation that gets cleaned up by the OS when the process terminates.

Why is WireShark checking for memory leaks during the configure phase? What's to be gained, there?
Comment 5 Dhiru Kholia 2015-10-29 09:52:01 UTC
http://clang.llvm.org/docs/AddressSanitizer.html#memory-leak-detection says,

"The leak detection is turned on by default on Linux; however, it is not yet supported on other platforms."

This is the reason (I think) why "./configure --enable-asan" fails in the Wireshark world.

It is possible to do "ASAN_OPTIONS=detect_leaks=0 ./configure --enable-asan" but that is a bit inconvenient, right? :-)

The one-off allocation is easy to free or eliminate altogether like *glib* itself did a while ago.
Comment 6 Peter Wu 2015-10-29 10:06:55 UTC
(In reply to Matthias Clasen from comment #2)
> it is really not our fault if wireshark thinks it is a good idea to check
> configure test programs for memory leaks and fail as a result...

All C(XX)FLAGS that are currently provided by the user (like -fsanitize=memory) are enabled at configure time. This is not heavy stuff like valgrind, but a compiler feature. Maybe it should only add this flag for user programs, but then maybe you actually want other C(XX)FLAGS to apply at configure time...

I suggest to apply the fix (static allocation) as it is trivial and will make all ASAN users of this m4 file happy.
Comment 7 Matthias Clasen 2015-10-30 01:00:00 UTC
Review of attachment 314259 [details] [review]:

Let's just take the rather than argue for weeks. It is also a cleanup, after all