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 522741 - regression: gst_index_entry_free() frees string it does not own
regression: gst_index_entry_free() frees string it does not own
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 0.10.18
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-03-16 11:47 UTC by Torsten Schoenfeld
Modified: 2008-03-17 10:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case (295 bytes, text/plain)
2008-03-16 11:47 UTC, Torsten Schoenfeld
  Details
minimal fix and unit test (3.60 KB, patch)
2008-03-16 14:55 UTC, Tim-Philipp Müller
committed Details | Review

Description Torsten Schoenfeld 2008-03-16 11:47:19 UTC
The entry returned by gst_index_add_format seems to have been freed already in current HEAD of CVS (0.10.17.3).  In 0.10.17 and earlier, the entry was owned by the caller and had to be freed.  Test case attached.  Tim suggested to classify this as a blocker.
Comment 1 Torsten Schoenfeld 2008-03-16 11:47:42 UTC
Created attachment 107381 [details]
Test case
Comment 2 Tim-Philipp Müller 2008-03-16 13:32:46 UTC
This seems to have been caused by this commit:

 2008-02-01  Julien Moutte  <julien@fluendo.com>

        * docs/gst/gstreamer-sections.txt: Add GST_CHECK_VERSION to the docs
        * gst/gstindex.c: (gst_index_class_init), (gst_index_free_writer),
        (gst_index_finalize), (gst_index_entry_free),
        (gst_index_add_association): Fix memory leaks.
        * gst/gstversion.h.in: Add GST_CHECK_VERSION macro.
        * plugins/indexers/gstmemindex.c: (gst_mem_index_class_init),
        (gst_mem_index_free_format), (gst_mem_index_free_id),
        (gst_mem_index_finalize): Fix memory leaks.
        * win32/common/config.h: Updated to CVS HEAD.

http://webcvs.freedesktop.org/gstreamer/gstreamer/gst/gstindex.c?r1=1.43&r2=1.44

The problem is the newly-added

 g_free (entry->data.format.key)

in line 555, which frees a string the index entry doesn't own (see line 590):

  const GstFormatDefinition *def;

  def = gst_format_get_details (format);
  entry->data.format.key = def->nick;

(This might theoretically also apply to data.object.key, but since that's not implemented it's not really an issue).

Comment 3 Tim-Philipp Müller 2008-03-16 14:55:01 UTC
Created attachment 107388 [details] [review]
minimal fix and unit test
Comment 4 Tim-Philipp Müller 2008-03-17 10:25:35 UTC
 2008-03-17  Tim-Philipp Müller  <tim at centricular dot net>

        * gst/gstindex.c: (gst_index_entry_free):
        * gst/gstindex.h:
          Don't free key strings which we don't own. Fixes crash in
          gst_index_entry_free() (#522741).

        * tests/check/Makefile.am:
        * tests/check/gst/.cvsignore:
        * tests/check/gst/gstindex.c: (test_index_entries),
          (gst_index_suite), (gst_index):
          Add unit test for the above.