What can you do? Sparta will need sons.

Tuesday, 23 December 2008

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

| 7 comments.

 

Comments On This Entry

[gravitar] TIm

Submitted at 04:44:44 on 23 december 2008

I've gone as far as debugging a couple of problems ... gdb is quite easy to use. Being a fairly new Debian user, I was eagerly waiting for the "make a patch" and "steps to help the developer get a fix in the archive steps" ... hope you can do chapter 2 of the public bug fix. By the way, what you are doing with these blogs is incredible: it shows people how easy it is to help. Very inspiring.
[author] Steve Kemp

Submitted at 04:50:35 on 23 december 2008

Thanks for the feedback!

Making patches is usually pretty easy, especially if you only make a couple of changes to a package. Mostly it comes down to keeping a copy of the original source, and then running:

diff --ignore-space-change --unified orig.file modified.file

But I'll make a note and try to work through another example in the next couple of days with more emphasis on what to do next..

[gravitar] Loris

Submitted at 07:57:16 on 23 december 2008

I like this kind of articles, they're very helpful. I'm looking forward to reading more about making patches and filing them. Thank you.
[gravitar] Jeremiah C. Foster

Submitted at 13:38:59 on 23 december 2008

Great stuff Steve! This kind of explication for hunting bugs is incredibly valuable! Thanks.
[gravitar] Faheem Mitha

Submitted at 16:45:20 on 23 december 2008

If you are using a version control system, it is painless to make a patch, and vc is useful in other ways. I like mercurial.
[author] Steve Kemp

Submitted at 16:57:09 on 23 december 2008

Indeed I'm a big fan of revision control systems - but generally I fix bugs by downloading the current source and working in place.

If I cannot fix a bug after an hour, or few hours, I give up. I've never yet needed to import the package source into a revision control system to work with it.

After all the pristine source is only an "apt-get source" away.

[author] Steve Kemp

Submitted at 17:22:44 on 23 december 2008

 

Comments are closed on posts which are more than ten days old.

Recent Posts

Recent Tags