Package: tagpy
Version: 0.91-1
Severity: normal
Tags: patch

Subject: tagpy: Fix segfault in tapgy.id3v2.Tag.addFrame()
Package: tagpy
Version: 0.91-1
Severity: important
Tags: patch

Adding a new Frame to an ID3v2 Tag reproducibly causes a segementation
fault. The TagLib documentation specifies that Tag::addFrame() takes
ownership of the pointer passed to it, but tagpy ignores this. I think
the result is that both, python and TagLib try to free() the Frame
pointer.

The crash is reproducible using the following python script:

import tagpy
import tagpy.id3v2

fileref = tagpy.FileRef('file.mp3')
file = fileref.file()
tag = file.ID3v2Tag(True)
frame = tagpy.id3v2.UniqueFileIdentifierFrame('blah', 'blah')
tag.addFrame(frame)
file.save()


According to the Boost.Python FAQ[1] the solution would be to use
std::auto_ptr. But I wasn't able to get it to work, so I worked around
using std::auto_ptr by simply cloning the Frame object before passing it
to addFrame().

Maybe somebody who is more knowledgeable in C++ than I am, can fix this
the right way.


[1] http://www.boost.org/libs/python/doc/v2/faq.html#ownership



-- System Information:
Debian Release: lenny/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (1, 'experimental')
Architecture: i386 (i686)

Kernel: Linux 2.6.22.2 (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

-- System Information:
Debian Release: lenny/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (1, 'experimental')
Architecture: i386 (i686)

Kernel: Linux 2.6.22.2 (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
--- orig/tagpy-0.91/src/id3.cpp 2006-03-21 19:08:11.000000000 +0100
+++ modified/tagpy-0.91/src/id3.cpp     2007-08-04 15:33:36.393646285 +0200
@@ -35,6 +35,12 @@
 
   void id3v2_Tag_removeFrame(ID3v2::Tag &t, ID3v2::Frame *f) { 
t.removeFrame(f); }
   
+  void id3v2_Tag_addFrame(ID3v2::Tag &t, ID3v2::Frame *f)
+  {
+    ID3v2::Frame *f_clone = 
ID3v2::FrameFactory::instance()->createFrame(f->render());
+    t.addFrame(f_clone);
+  }
+ 
   object id3v2_rvf_channels(const ID3v2::RelativeVolumeFrame &rvf)
   {
     List<ID3v2::RelativeVolumeFrame::ChannelType> l = rvf.channels();
@@ -171,7 +177,7 @@
       .def("frameList", fl1, return_internal_reference<>())
       .def("frameList", fl2, return_internal_reference<>())
       
-      .DEF_SIMPLE_METHOD(addFrame)
+      .def("addFrame", &id3v2_Tag_addFrame)
       .DEF_SIMPLE_METHOD(removeFrame)
       .DEF_SIMPLE_METHOD(removeFrames)
       

Reply via email to