Since I've ranted a little recently lets do another public bugfix. The last few times people seemed to like them, and writing things down helps me keep track of where I am, what I'm doing, and how soon it will be "beer o'clock".
So I looked over the release critical bug list, looking for things that might be easy to fix.
One bug jumped out at me:
I installed the package:
skx@gold:~$ apt-get install gnomad2 .. The following NEW packages will be installed gnomad2 libmtp7 libnjb5 libtagc0 ..
Once I copied an .mp3 file to my home directory, with the .ogg suffix I got a segfault on startup:
skx@gold:~$ cp /home/music/Audio/RedDwarf-back-to-reality.mp3 ~/foo.ogg skx@gold:~$ gnomad2 .. LIBMTP_Get_First_Device: No Devices Attached PDE device NULL. TagLib: Ogg::File::packet() -- Could not find the requested packet. TagLib: Vorbis::File::read() - Could not find the Vorbis comment header. [segfault] skx@gold:~$
So I downloaded the source ("apt-get source gnomad2"), and the dependencies for rebuilding ("apt-get build-dep gnomad2"). This allowed me to rebuild it locally:
skx@gold:/tmp/gnomad2-2.9.1$ ./configure --enable-debug && make skx@gold:/tmp/gnomad2-2.9.1$ cd src/
And now it can be run under GDB.
skx@gold:/tmp/gnomad2-2.9.1/src$ gdb ./gnomad2 GNU gdb 6.8-debian ... (gdb) run Starting program: /tmp/gnomad2-2.9.1/src/gnomad2 ... LIBMTP_Get_First_Device: No Devices Attached PDE device NULL. [New Thread 0x41dcd950 (LWP 23593)] TagLib: Ogg::File::packet() -- Could not find the requested packet. TagLib: Vorbis::File::read() - Could not find the Vorbis comment header. Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x41dcd950 (LWP 23593)] 0x00007fc89e0340b0 in taglib_tag_artist () from /usr/lib/libtag_c.so.0 (gdb)
Interestingly the crash comes from a library, libtag_c.so.0. So either:
- The bug is realy in the libtag module.
- The gnomad2 package doesn't handle a failure case it should.
Time to guess which that is. Charitably we'll assume any library that segfaults will be quickly fixed, because it will have more users than a single program..
Moving on we can look at the gnomad2 source for mentions of taglib. Several mentions are found, but src/tagfile.c is clearly the correct place to look. That file contains the following code:
void
get_tag_for_file (metadata_t *meta)
{
gchar *tmppath = filename_fromutf8(meta->path);
TagLib_Tag *tag;
const TagLib_AudioProperties *properties;
TagLib_File *file = taglib_file_new(tmppath);
if(file == NULL) {
g_printf("could not open file %s", tmppath);
g_free(tmppath);
return;
}
g_free(tmppath);
tag = taglib_file_tag(file);
properties = taglib_file_audioproperties(file);
gchar* artist = taglib_tag_artist(tag);
..
This looks like a great place to explore because opening a file to read the tags is most likely where the crash is going to be coming from.
Interestingly we see the code:
- Calls taglib_file_new to get a handle of some kind relating to the file.
- Tests for errors with that operation.
- Calls taglib_file_tag to fetch tag information using the handle, and indirectly the file.
- But then uses taglib_tag_artist to fetch the artist (?I guess?) without testing the handle previously obtained was valid.
Let us guess that the file opening is succeeding but that the tag structure fetched via taglib_file_tag is NULL - and this causes the crash.
A quick edit:
g_free(tmppath);
tag = taglib_file_tag(file);
if(tag == NULL) {
g_printf("tag was null");
return;
}
properties = taglib_file_audioproperties(file);
gchar* artist = taglib_tag_artist(tag);
Rebuild, and the segfault is gone. We have a winner. Now we just need to file a patch...
ObFilm: 300
Tags: debian, gnomad2, public bugfixing 7 comments
