|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>|
|Severity:||Affects Some People||Keywords:||needs-qa, patch|
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 2 Kubilay Kocak 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 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 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)  - Bump PORTREVISION on dependent ports PR: 196959  Submitted by: Pete Johanson <email@example.com>  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 2015-06-14 17:58:17 UTC
Committed. Thanks for your work and sorry it took so long!