1. By moving the 'return 0;' inside the cpp clause an unreachable defect is not
triggered.
2. If a parent isn't found then ripTrack could be leaked.
3. item declaration hides the item parameter in the member function.
4. Declaration of outfile hides parameter in ctor.
5. Since the else clause guarantees that the function will end the second
'return -1;' is not necessary. If the else clause is just moved to the
end and the extra return removed it has the same effect.
6. Using sprintf with getenv is dangerous and could cause the string to be
overflowed. Switching to snprintf and saving the string from the
previous getenv call.
7. m_decoder is the wrong pointer to check. The one to check is the one given
by the dyn cast.
8. else return missing from conditional clause. Without it there then node could
be dereferenced even though it is invalid (and a segfault could happen).
9. tmpdata might be null and then dereferenced when getting album and artist.
10. Use of tmpnam is dangerous. Using createTempFile from libmyth.
11. PLField might be null. Dereferencing it could end in segfault.
12. Assignment of item in do/while loop is unnecessary.
13. Removal of dead code.
Signed-off-by: Erik Hovland <erik@hovland.org>
---
mythplugins/mythmusic/mythmusic/cddecoder.cpp | 1
mythplugins/mythmusic/mythmusic/cdrip.cpp | 6 +-
mythplugins/mythmusic/mythmusic/databasebox.cpp | 28 +++++----
mythplugins/mythmusic/mythmusic/encoder.cpp | 3 -
mythplugins/mythmusic/mythmusic/filescanner.cpp | 6 --
mythplugins/mythmusic/mythmusic/main.cpp | 6 +-
mythplugins/mythmusic/mythmusic/musicplayer.cpp | 4 +
mythplugins/mythmusic/mythmusic/playlist.cpp | 69 ++++++++++++---------
mythplugins/mythmusic/mythmusic/smartplaylist.cpp | 12 ++--
mythplugins/mythmusic/mythmusic/visualize.cpp | 6 --
10 files changed, 72 insertions(+), 69 deletions(-)
diff --git a/mythplugins/mythmusic/mythmusic/cddecoder.cpp b/mythplugins/mythmusic/mythmusic/cddecoder.cpp
index c90f391..2efc054 100644
a
|
b
|
Decoder *CdDecoderFactory::create(const QString &file, QIODevice *input, |
580 | 580 | |
581 | 581 | return decoder; |
582 | 582 | } |
583 | | |
diff --git a/mythplugins/mythmusic/mythmusic/cdrip.cpp b/mythplugins/mythmusic/mythmusic/cdrip.cpp
index 91e0de3..287c935 100644
a
|
b
|
int CDRipperThread::ripTrack(QString &cddevice, Encoder *encoder, int tracknum) |
410 | 410 | return (curpos - start + 1) * CD_FRAMESIZE_RAW; |
411 | 411 | #else |
412 | 412 | (void)cddevice; (void)encoder; (void)tracknum; |
413 | | #endif |
414 | | |
415 | 413 | return 0; |
| 414 | #endif |
416 | 415 | } |
417 | 416 | |
418 | 417 | /////////////////////////////////////////////////////////////////////////////// |
… |
… |
void Ripper::startScanCD(void) |
783 | 782 | } |
784 | 783 | } |
785 | 784 | } |
786 | | } |
| 785 | } else |
| 786 | delete ripTrack; |
787 | 787 | } |
788 | 788 | |
789 | 789 | m_artistEdit->setText(m_artistName); |
diff --git a/mythplugins/mythmusic/mythmusic/databasebox.cpp b/mythplugins/mythmusic/mythmusic/databasebox.cpp
index a58d82d..62210ad 100644
a
|
b
|
void DatabaseBox::deleteTrack(UIListGenericTree *item) |
1082 | 1082 | else if (delete_item->prevSibling(1)) |
1083 | 1083 | tree->MoveUp(); |
1084 | 1084 | |
1085 | | UIListGenericTree *item = (UIListGenericTree *)delete_item->getParent(); |
1086 | | if (TreeCheckItem *item_owner = dynamic_cast<TreeCheckItem*>(item)) |
| 1085 | UIListGenericTree *newItem = (UIListGenericTree *)delete_item->getParent(); |
| 1086 | if (TreeCheckItem *item_owner = dynamic_cast<TreeCheckItem*>(newItem)) |
1087 | 1087 | { |
1088 | | Playlist *owner = gMusicData->all_playlists->getPlaylist(item_owner->getID() * |
1089 | | -1); |
1090 | | owner->removeTrack(delete_item->getID(), true); |
| 1088 | Playlist *owner = |
| 1089 | gMusicData->all_playlists->getPlaylist(item_owner->getID() * -1); |
| 1090 | |
| 1091 | if (owner) |
| 1092 | owner->removeTrack(delete_item->getID(), true); |
1091 | 1093 | } |
1092 | | else if (PlaylistTitle *item_owner = dynamic_cast<PlaylistTitle*>(item)) |
| 1094 | else if (PlaylistTitle *item_owner = dynamic_cast<PlaylistTitle*>(newItem)) |
1093 | 1095 | { |
1094 | 1096 | (void)item_owner; |
1095 | 1097 | active_playlist->removeTrack(delete_item->getID(), true); |
… |
… |
void DatabaseBox::deleteTrack(UIListGenericTree *item) |
1110 | 1112 | else if (delete_item->prevSibling(1)) |
1111 | 1113 | tree->MoveUp(); |
1112 | 1114 | |
1113 | | UIListGenericTree *item = (UIListGenericTree *)delete_item->getParent(); |
1114 | | if (TreeCheckItem *item_owner = dynamic_cast<TreeCheckItem*>(item)) |
| 1115 | UIListGenericTree *newItem = (UIListGenericTree *)delete_item->getParent(); |
| 1116 | if (TreeCheckItem *item_owner = dynamic_cast<TreeCheckItem*>(newItem)) |
1115 | 1117 | { |
1116 | | Playlist *owner = gMusicData->all_playlists->getPlaylist(item_owner->getID() * |
1117 | | -1); |
1118 | | owner->removeTrack(delete_item->getID(), false); |
| 1118 | Playlist *owner = |
| 1119 | gMusicData->all_playlists->getPlaylist(item_owner->getID() * -1); |
| 1120 | |
| 1121 | if (owner) |
| 1122 | owner->removeTrack(delete_item->getID(), false); |
1119 | 1123 | } |
1120 | | else if (PlaylistTitle *item_owner = dynamic_cast<PlaylistTitle*>(item)) |
| 1124 | else if (PlaylistTitle *item_owner = dynamic_cast<PlaylistTitle*>(newItem)) |
1121 | 1125 | { |
1122 | 1126 | (void)item_owner; |
1123 | 1127 | active_playlist->removeTrack(delete_item->getID(), false); |
diff --git a/mythplugins/mythmusic/mythmusic/encoder.cpp b/mythplugins/mythmusic/mythmusic/encoder.cpp
index 19c259e..a590d2a 100644
a
|
b
|
Encoder::Encoder(const QString &outfile, int qualitylevel, Metadata *metadata) |
13 | 13 | { |
14 | 14 | if (!m_outfile.isEmpty()) |
15 | 15 | { |
16 | | QByteArray outfile = m_outfile.toLocal8Bit(); |
17 | | m_out = fopen(outfile.constData(), "w+"); |
| 16 | m_out = fopen((m_outfile.toLocal8Bit()).constData(), "w+"); |
18 | 17 | if (!m_out) |
19 | 18 | { |
20 | 19 | VERBOSE(VB_GENERAL, QString("Error opening output file: '%1'") |
diff --git a/mythplugins/mythmusic/mythmusic/filescanner.cpp b/mythplugins/mythmusic/mythmusic/filescanner.cpp
index 40ceacf..ed5e7f7 100644
a
|
b
|
int FileScanner::GetDirectoryId(const QString &directory, const int &parentid) |
194 | 194 | return query.lastInsertId().toInt(); |
195 | 195 | } |
196 | 196 | } |
197 | | else |
198 | | { |
199 | | MythContext::DBError("music select directory id", query); |
200 | | return -1; |
201 | | } |
202 | 197 | |
| 198 | MythContext::DBError("music select directory id", query); |
203 | 199 | return -1; |
204 | 200 | } |
205 | 201 | |
diff --git a/mythplugins/mythmusic/mythmusic/main.cpp b/mythplugins/mythmusic/mythmusic/main.cpp
index a1385e7..7bd3411 100644
a
|
b
|
QString chooseCD(void) |
59 | 59 | void CheckFreeDBServerFile(void) |
60 | 60 | { |
61 | 61 | char filename[1024]; |
62 | | if (getenv("HOME") == NULL) |
| 62 | char* home_string = NULL; |
| 63 | if ((home_string = getenv("HOME")) == NULL) |
63 | 64 | { |
64 | 65 | VERBOSE(VB_IMPORTANT, "main.o: You don't have a HOME environment variable. CD lookup will almost certainly not work."); |
65 | 66 | return; |
66 | 67 | } |
67 | | sprintf(filename, "%s/.cdserverrc", getenv("HOME")); |
| 68 | snprintf(filename, 1023, "%s/.cdserverrc", home_string); |
| 69 | filename[1023] = '\0'; |
68 | 70 | |
69 | 71 | QFile file(filename); |
70 | 72 | |
diff --git a/mythplugins/mythmusic/mythmusic/musicplayer.cpp b/mythplugins/mythmusic/mythmusic/musicplayer.cpp
index 7942c0e..303f120 100644
a
|
b
|
void MusicPlayer::play(void) |
311 | 311 | { |
312 | 312 | // CD track |
313 | 313 | CdDecoder *cddecoder = dynamic_cast<CdDecoder*>(m_decoder); |
314 | | if (m_decoder) |
| 314 | if (cddecoder) |
315 | 315 | m_currentMetadata = cddecoder->getMetadata(-m_currentNode->getInt()); |
316 | 316 | } |
317 | 317 | } |
… |
… |
void MusicPlayer::next(void) |
397 | 397 | else |
398 | 398 | return; // stop() |
399 | 399 | } |
| 400 | else |
| 401 | return; // stop() |
400 | 402 | } |
401 | 403 | else |
402 | 404 | return; // stop() |
diff --git a/mythplugins/mythmusic/mythmusic/playlist.cpp b/mythplugins/mythmusic/mythmusic/playlist.cpp
index 6fc7d7c..bb05c20 100644
a
|
b
|
int Playlist::writeTree(GenericTree *tree_to_write_to, int a_counter) |
972 | 972 | // Normal track |
973 | 973 | Metadata *tmpdata |
974 | 974 | = all_available_music->getMetadata(it->getValue()); |
975 | | if (tmpdata && tmpdata->isVisible()) |
| 975 | if (tmpdata) |
976 | 976 | { |
977 | | if (songs.at() == 0) |
978 | | { // first song |
979 | | playcountMin = playcountMax = tmpdata->PlayCount(); |
980 | | lastplayMin = lastplayMax = tmpdata->LastPlay(); |
981 | | } |
982 | | else |
| 977 | if (tmpdata->isVisible()) |
983 | 978 | { |
984 | | if (tmpdata->PlayCount() < playcountMin) |
985 | | playcountMin = tmpdata->PlayCount(); |
986 | | else if (tmpdata->PlayCount() > playcountMax) |
987 | | playcountMax = tmpdata->PlayCount(); |
988 | | |
989 | | if (tmpdata->LastPlay() < lastplayMin) |
990 | | lastplayMin = tmpdata->LastPlay(); |
991 | | else if (tmpdata->LastPlay() > lastplayMax) |
992 | | lastplayMax = tmpdata->LastPlay(); |
| 979 | if (songs.at() == 0) |
| 980 | { // first song |
| 981 | playcountMin = playcountMax = tmpdata->PlayCount(); |
| 982 | lastplayMin = lastplayMax = tmpdata->LastPlay(); |
| 983 | } |
| 984 | else |
| 985 | { |
| 986 | if (tmpdata->PlayCount() < playcountMin) |
| 987 | playcountMin = tmpdata->PlayCount(); |
| 988 | else if (tmpdata->PlayCount() > playcountMax) |
| 989 | playcountMax = tmpdata->PlayCount(); |
| 990 | |
| 991 | if (tmpdata->LastPlay() < lastplayMin) |
| 992 | lastplayMin = tmpdata->LastPlay(); |
| 993 | else if (tmpdata->LastPlay() > lastplayMax) |
| 994 | lastplayMax = tmpdata->LastPlay(); |
| 995 | } |
993 | 996 | } |
| 997 | // pre-fill the album-map with the album name. |
| 998 | // This allows us to do album mode in album order |
| 999 | album = tmpdata->Album(); |
| 1000 | // pre-fill the artist map with the artist name and song title |
| 1001 | artist = tmpdata->Artist() + "~" + tmpdata->Title(); |
994 | 1002 | } |
995 | | // pre-fill the album-map with the album name. |
996 | | // This allows us to do album mode in album order |
997 | | album = tmpdata->Album(); |
998 | 1003 | if ((Ialbum = album_map.find(album)) == album_map.end()) |
999 | | { |
1000 | 1004 | album_map.insert(AlbumMap::value_type(album,0)); |
1001 | | } |
1002 | 1005 | |
1003 | | // pre-fill the artist map with the artist name and song title |
1004 | | artist = tmpdata->Artist() + "~" + tmpdata->Title(); |
1005 | 1006 | if ((Iartist = artist_map.find(artist)) == artist_map.end()) |
1006 | | { |
1007 | | artist_map.insert(ArtistMap::value_type(artist,0)); |
1008 | | } |
| 1007 | artist_map.insert(ArtistMap::value_type(artist,0)); |
1009 | 1008 | } |
1010 | 1009 | } |
1011 | 1010 | } |
… |
… |
int Playlist::CreateCDMP3(void) |
1776 | 1775 | } |
1777 | 1776 | |
1778 | 1777 | // probably should tie stdout of mkisofs to stdin of cdrecord sometime |
1779 | | char tmprecordlist[L_tmpnam]; |
1780 | | char tmprecordisofs[L_tmpnam]; |
| 1778 | QString tmptemplate("/tmp/mythmusicXXXXXX"); |
1781 | 1779 | |
1782 | | tmpnam(tmprecordlist); |
1783 | | tmpnam(tmprecordisofs); |
| 1780 | QString tmprecordlist = createTempFile(tmptemplate); |
| 1781 | if (tmprecordlist == tmptemplate) |
| 1782 | { |
| 1783 | VERBOSE(VB_IMPORTANT, "Unable to open temporary file"); |
| 1784 | return 1; |
| 1785 | } |
| 1786 | |
| 1787 | QString tmprecordisofs = createTempFile(tmptemplate); |
| 1788 | if (tmprecordisofs == tmptemplate) |
| 1789 | { |
| 1790 | VERBOSE(VB_IMPORTANT, "Unable to open temporary file"); |
| 1791 | return 1; |
| 1792 | } |
1784 | 1793 | |
1785 | 1794 | QFile reclistfile(tmprecordlist); |
1786 | 1795 | |
diff --git a/mythplugins/mythmusic/mythmusic/smartplaylist.cpp b/mythplugins/mythmusic/mythmusic/smartplaylist.cpp
index 95c06b5..a3a3603 100644
a
|
b
|
void SmartPLCriteriaRow::initValues(QString Field, QString Operator, QString Val |
826 | 826 | fieldCombo->setCurrentText(Field); |
827 | 827 | operatorCombo->setCurrentText(Operator); |
828 | 828 | |
829 | | SmartPLField *PLField; |
830 | | PLField = lookupField(Field); |
| 829 | SmartPLField *PLField = lookupField(Field); |
831 | 830 | if (PLField) |
832 | 831 | { |
833 | 832 | if (PLField->type == ftNumeric) |
… |
… |
void SmartPLCriteriaRow::initValues(QString Field, QString Operator, QString Val |
853 | 852 | } |
854 | 853 | else |
855 | 854 | { |
856 | | value1SpinEdit->setValue(PLField->defaultValue); |
857 | | value2SpinEdit->setValue(PLField->defaultValue); |
| 855 | value1SpinEdit->setValue(0); |
| 856 | value2SpinEdit->setValue(0); |
858 | 857 | value1Edit->setText(""); |
859 | 858 | value2Edit->setText(""); |
860 | 859 | } |
… |
… |
void SmartPLResultViewer::setSQL(QString sql) |
1733 | 1732 | MSqlQuery query(MSqlQuery::InitCon()); |
1734 | 1733 | query.exec(sql); |
1735 | 1734 | |
1736 | | Q3ListViewItem *item; |
1737 | 1735 | if (query.last()) |
1738 | 1736 | { |
1739 | 1737 | do |
1740 | 1738 | { |
1741 | | item = new Q3ListViewItem(listView, |
| 1739 | new Q3ListViewItem(listView, |
1742 | 1740 | query.value(0).toString(), |
1743 | 1741 | query.value(1).toString(), |
1744 | 1742 | query.value(2).toString(), |
… |
… |
void SmartPLResultViewer::setSQL(QString sql) |
1750 | 1748 | } |
1751 | 1749 | |
1752 | 1750 | // set selection to first item |
1753 | | item = listView->firstChild(); |
| 1751 | Q3ListViewItem *item = listView->firstChild(); |
1754 | 1752 | if (item) |
1755 | 1753 | listView->setSelected(item, true); |
1756 | 1754 | } |
diff --git a/mythplugins/mythmusic/mythmusic/visualize.cpp b/mythplugins/mythmusic/mythmusic/visualize.cpp
index cba922f..8f8ca9e 100644
a
|
b
|
bool Squares::draw(QPainter *p, const QColor &back) |
571 | 571 | #endif |
572 | 572 | |
573 | 573 | return true; |
574 | | |
575 | | for (int x = 0; x < size.width (); x += w) { |
576 | | p->fillRect (x, center - h / 2, w, h, QColor ("red")); |
577 | | p->fillRect (x, center + h / 2, w, h, QColor ("blue")); |
578 | | } |
579 | | return true; |
580 | 574 | } |
581 | 575 | |
582 | 576 | static class SquaresFactory : public VisFactory |