Taglib segfaults - Null String #296
Conversation
Taglib strings can be NULL and using it could cause some segfaults, so in this case it will return a QString()
- Try converting a TString from a metadata directly to QString can produce segfaults. - We must check if it is valid before convert it.
|
Nice pick! LGTM Thank you! |
| @@ -223,6 +223,13 @@ void SoundSource::setKey(QString key){ | |||
| m_sKey = key; | |||
| } | |||
|
|
|||
| QString SoundSource::toQString(TagLib::String tstring) const { | |||
| if (tstring != TagLib::String::null) { | |||
There was a problem hiding this comment.
total nitpick: can you say if (tstring == null) { return QString() } since that's the special case? I just like the look of it more that way :)
There was a problem hiding this comment.
Do you mean define TagLib::String::null as NULL ?
There was a problem hiding this comment.
Like this
if (tstring == TagLib::String::null) {
return QString();
}
return TStringToQString(tstring);I don't know if this is in the coding guidelines but usually in mixxx we exit function early only when an error occurs. The normal everything went ok exist is always at the end. In this case tstring being null is the error
@kain88-de: I don't know if this is in the coding guidelines but usually in mixxx we exit function early only when an error occurs. The normal everything went ok exist is always at the end. In this case tstring being null is the error
| @@ -223,6 +223,13 @@ void SoundSource::setKey(QString key){ | |||
| m_sKey = key; | |||
| } | |||
|
|
|||
| QString SoundSource::toQString(TagLib::String tstring) const { | |||
| if (tstring == TagLib::String::null) { | |||
| return QString(); | |||
There was a problem hiding this comment.
extra space between return and QString
There was a problem hiding this comment.
ops, sorry...
done! =D
|
@daschuer keep in mind this will break the windows build until we update taglib |
|
@daschuer merging this will update the min required taglib version. Ubuntu 12.04 uses 1.7.1 a minor above the version we use in the windows build. @cardinot what is the min required taglib version for your changes to work? I don't have any problems merging this and saying that we don't support the old stable of ubuntu anymore. |
Version 1.8. |
|
Why does this patch not work with TagLib 1.7? |
Because 1.7 does not have a method with the operator !=... |
|
However, the version 1.7 has the operator== and I could use it to let it work with 1.7... ? |
|
You are already using I can test it on windows sometime next week again. If it builds I will merge it |
|
damn! I had forgotten that I had changed it already... sorry! =( |
|
Yeah, it works on Windows 7 (with the prebuild package)! |
|
awesome, thanks |
Taglib segfaults - Null String
|
thanks for picking this up! |
Add Download links to the beta version
Recently I faced up to some crashes due to metadata extraction... mainly when I'm trying to read a lot of embedded_covers at the same time. I realized that Mixxx is getting the info directly from the tag and trying to convert to QString and it was also the cause of the crashes.