Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guard against undefined std::string behavior. #6

Merged
merged 1 commit into from
Jan 21, 2015

Conversation

petejohanson
Copy link
Contributor

This came out of a discussion of a crash in sound-juicer on FreeBSD 11.0-CURRENT on the GNOME bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=742019 The current code happens to work on Linux, but this is a coincidence, and not based on defined behaviour, AFAICT.

I think it is fair that libmusicbrainz should guard against this so the C interface can be idiomatic, but libmusicbrainz avoids the undefined behaviour internally.

Thoughts?

* Passing NULL char pointer to std::string constructor results
  in undefined behaviour, so guard against that in the generated
  C interface. See:

  http://stackoverflow.com/questions/17464514/initialize-stdstring-from-a-possibly-null-char-pointer
@petejohanson
Copy link
Contributor Author

Any thoughts?

@adhawkins
Copy link
Member

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.

@petejohanson
Copy link
Contributor Author

Ok, I've added your comments to the SJ bug (https://bugzilla.gnome.org/show_bug.cgi?id=742019) to get their feedback. I've also created a FreeBSD PR (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196959) for this to address this in FreeBSD until a new libmusicbrainz5 version is released.

Thanks.

adhawkins added a commit that referenced this pull request Jan 21, 2015
Guard against undefined std::string behavior.
@adhawkins adhawkins merged commit 1e011ae into metabrainz:master Jan 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants