Bug 196959

Summary: audio/libmusicbrainz5: Use of undefined std:string behaviour causes sound-juicer crash
Product: Ports & Packages Reporter: Pete Johanson <peter>
Component: Individual Port(s)Assignee: Jason E. Hale <jhale>
Status: Closed FIXED    
Severity: Affects Some People Keywords: needs-qa, patch
Priority: --- Flags: bugzilla: maintainer-feedback? (jhale)
Version: Latest   
Hardware: Any   
OS: Any   
See Also: https://bugzilla.gnome.org/show_bug.cgi?id=742019
Attachments:
Description Flags
Patch to audio/libmusicbrainz5 port none

Description Pete Johanson 2015-01-21 03:22:05 UTC
In diagnosing an issue w/ Sound Juicer (audio/sound-juicer) crashing on startup on FreeBSD-11.0-CURRENT (https://bugzilla.gnome.org/show_bug.cgi?id=742019), research ultimately showed that the crash was thanks to libmusicbrainz5 depending on undefined behaviour of the std::string constructor which happens to work on Linux. (it is passing NULL to the std::string constructor, instead of "")

I have opened a GitHub PR w/ the libmusicbrainz5 project: https://github.com/metabrainz/libmusicbrainz/pull/6

But so far have gotten no response. With the change in that pull request, Sound Juicer starts and runs as expected on my system.

I'm a relative newbie to FreeBSD, is the above change something that would normally get included in the FreeBSD port until upstream fixes and puts out a new release?

Thanks in advance.
Comment 1 Bugzilla Automation freebsd_committer 2015-01-21 03:22:05 UTC
Auto-assigned to maintainer jhale@FreeBSD.org
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2015-01-21 03:32:25 UTC
It is absolutely normal to include local patches while upstream gets them merged.

If you can include an svn diff against the latest version of the port and attach it here that would be great.

Nice submission Peter, good work on the upstreaming too :)
Comment 3 Pete Johanson 2015-01-21 04:49:15 UTC
Created attachment 151943 [details]
Patch to audio/libmusicbrainz5 port

Lord my svn fu is rusty, this is a svn diff of just the port directory.

If some other format is required, please let me know.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2015-01-21 05:38:14 UTC
Normally the svn diff is done from /usr/ports for more changes covering multiple ports/paths, or from /usr/ports/category/port for individual ports, the version you attached is fine though, thank you :)
Comment 5 Pete Johanson 2015-01-21 16:27:05 UTC
Well, this is from one of the libmusicbrainz developers on the GH PR:

"I'm happy to merge this, but can't see me releasing a new version simply to fix this.

To be honest, I think it's quite clear that the C interface is expecting a null terminated string here, so ultimately the calling application should be responsible for passing correct data. I'd suggest an update to sound-juicer to ensure that it passes an empty string (or doesn't call the method at all) if no proxy is required."

I've commented on the original sound-juicer bug, not sure which way this will fall (although ultimately it does sound like the fix attached here is going to go into libmusicbrainz5), not sure how FreeBSD wants to handle this in the meantime. I can just as easily submit a patch for sound-juicer to update it's using of the libmusicbrainz5 API to use null terminated strings instead of NULL.
Comment 6 Pete Johanson 2015-01-21 19:52:55 UTC
The GH pull request was merged into libmusicbrainz, so regardless of how the sound-juicer devs decide to handle this, I think this is a good candidate for inclusion in the ports tree until a new libmusicbrainz is released.
Comment 7 commit-hook freebsd_committer 2015-06-14 17:56:25 UTC
A commit references this bug:

Author: jhale
Date: Sun Jun 14 17:55:53 UTC 2015
New revision: 389646
URL: https://svnweb.freebsd.org/changeset/ports/389646

Log:
  - Update audio/libmusicbrainz5 to 5.1.0
  - Fix a crash in audio/sound-juicer caused by libmusicbrainz5 (fix added
    upstream, but not in this release) [1]
  - Bump PORTREVISION on dependent ports

  PR:		196959 [1]
  Submitted by:	Pete Johanson <peter@peterjohanson.com> [1]

Changes:
  head/UPDATING
  head/audio/cantata/Makefile
  head/audio/goobox/Makefile
  head/audio/libkcddb/Makefile
  head/audio/libmusicbrainz5/Makefile
  head/audio/libmusicbrainz5/distinfo
  head/audio/libmusicbrainz5/files/
  head/audio/libmusicbrainz5/files/patch-src_c-int-source-funcs.inc
  head/audio/libmusicbrainz5/pkg-plist
  head/audio/rhythmbox/Makefile
  head/audio/sound-juicer/Makefile
  head/deskutils/nemo-extensions/Makefile
  head/x11-fm/sushi/Makefile
Comment 8 Jason E. Hale freebsd_committer 2015-06-14 17:58:17 UTC
Committed.  Thanks for your work and sorry it took so long!