xaizek / pms (License: GPLv3+) (since 2018-12-07)
Older version of Practical Music Search written in C++.
Commit 0cf7f4c0b16a038751f5b0987b70f56295ab063d

Fix removal of songs in the queue, and fix memory leak in Songlist::add()
Author: Kim Tore Jensen
Author date (UTC): 2015-08-22 08:55
Committer name: Kim Tore Jensen
Committer date (UTC): 2015-08-22 08:55
Parent(s): d18d0941e7447536dadef06802d9b35e368d02d3
Signing key:
Tree: ca24dbbbc524a919da3ea16de3c487b1ab7a9e7f
File Lines added Lines deleted
src/action.cpp 15 22
src/command.cpp 17 23
src/command.h 1 1
src/list.cpp 33 23
src/list.h 2 2
File src/action.cpp changed (mode: 100644) (index 2d87cbe..4fef9d6)
... ... long Interface::crop(int crop_mode)
977 977 /* /*
978 978 * Remove selected songs from list * Remove selected songs from list
979 979 */ */
980 long Interface::remove(Songlist * list)
980 long
981 Interface::remove(Songlist * list)
981 982 { {
982 int count = 0;
983 983 Song * song; Song * song;
984 984 vector<Song *> songs; vector<Song *> songs;
985 985 vector<Song *>::iterator i; vector<Song *>::iterator i;
986 986
987 if (!list)
988 {
987 /* FIXME: this check should, perhaps, be done earlier? */
988 if (!list) {
989 989 pms->log(MSG_STATUS, STERR, _("This is not a playlist: you can't remove songs from here.")); pms->log(MSG_STATUS, STERR, _("This is not a playlist: you can't remove songs from here."));
990 990 return STERR; return STERR;
991 991 } }
992 992
993 if (list == pms->comm->library())
994 {
993 /* FIXME: same goes for this check */
994 if (list == pms->comm->library()) {
995 995 pms->log(MSG_STATUS, STERR, _("The library is read-only.")); pms->log(MSG_STATUS, STERR, _("The library is read-only."));
996 996 return STERR; return STERR;
997 997 } }
998 998
999 song = list->popnextselected();
1000 while (song != NULL)
1001 {
999 while ((song = list->popnextselected()) != NULL) {
1002 1000 songs.push_back(song); songs.push_back(song);
1003 song = list->popnextselected();
1004 1001 } }
1005 1002
1006 1003 i = songs.begin(); i = songs.begin();
1007 while (i != songs.end())
1008 {
1009 if (pms->comm->remove(list, *i))
1010 ++count;
1004 while (i != songs.end()) {
1005 if (!pms->comm->remove(list, *i)) {
1006 return STERR;
1007 }
1011 1008 ++i; ++i;
1012 1009 } }
1013 1010
1014 if (count > 0)
1015 {
1016 pms->disp->actwin()->wantdraw = true;
1017 pms->log(MSG_STATUS, STOK, _("Removed %d %s."), count, (count == 1 ? _("song") : _("songs")));
1018 return STOK;
1019 }
1011 /* FIXME: should wantdraw really be set _here_? */
1012 pms->disp->actwin()->wantdraw = true;
1013 pms->log(MSG_STATUS, STOK, _("Removed %d %s."), songs.size(), (songs.size() == 1 ? _("song") : _("songs")));
1020 1014
1021 pms->log(MSG_STATUS, STERR, _("No songs removed."));
1022 return STERR;
1015 return STOK;
1023 1016 } }
1024 1017
1025 1018 /* /*
File src/command.cpp changed (mode: 100644) (index 7051fcb..18eab10)
... ... Control::add(Songlist * list, Song * song)
807 807 /* /*
808 808 * Remove a song from the playlist * Remove a song from the playlist
809 809 */ */
810 int
810 bool
811 811 Control::remove(Songlist * list, Song * song) Control::remove(Songlist * list, Song * song)
812 812 { {
813 813 int pos = MATCH_FAILED; int pos = MATCH_FAILED;
814 814
815 815 assert(song != NULL); assert(song != NULL);
816 816 assert(list != NULL); assert(list != NULL);
817 assert(list != _library);
817 818
818 if (list == _library) {
819 // FIXME: error message
820 return false;
821 }
819 EXIT_IDLE;
820
821 pms->log(MSG_DEBUG, 0, "Removing song with id=%d pos=%d uri=%s from list %s.\n", song->id, song->pos, song->file.c_str(), list->filename.c_str());
822 822
823 if (list == _playlist && song->id == MPD_SONG_NO_ID) {
823 /* Remove song from queue */
824 if (list == _playlist) {
824 825 // All songs must have ID's // All songs must have ID's
825 826 // FIXME: version requirement // FIXME: version requirement
826 827 assert(song->id != MPD_SONG_NO_ID); assert(song->id != MPD_SONG_NO_ID);
828 return mpd_run_delete_id(conn->h(), song->id);
827 829 } }
828 830
829 if (list != _playlist) {
830 if (list->filename.size() == 0) {
831 // FIXME: what does this check?
832 return false;
833 }
834 pos = list->locatesong(song);
835 if (pos == MATCH_FAILED) {
836 // FIXME: error message
837 return false;
838 }
839 pms->log(MSG_DEBUG, 0, "Removing song %d from list.\n", pos);
840 }
831 /* Remove song from stored playlist */
832 assert(list->filename.size() == 0);
841 833
842 if (list == _playlist) {
843 return mpd_run_delete_id(conn->h(), song->id);
844 } else {
845 return mpd_run_playlist_delete(conn->h(), (char *)list->filename.c_str(), pos);
834 pos = list->locatesong(song);
835 if (pos == MATCH_FAILED) {
836 // FIXME: error message
837 return false;
846 838 } }
847 839
840 return mpd_run_playlist_delete(conn->h(), (char *)list->filename.c_str(), pos);
841
848 842 // FIXME: remove from list? // FIXME: remove from list?
849 843 /* /*
850 844 if (command_mode != 0) return true; if (command_mode != 0) return true;
 
... ... bool Control::activatelist(Songlist * list)
1653 1647
1654 1648 /* /*
1655 1649 * Retrieves current playlist from MPD * Retrieves current playlist from MPD
1656 * TODO: implement more entity types
1650 * TODO: implement missing entity types
1657 1651 */ */
1658 1652 bool bool
1659 1653 Control::update_queue() Control::update_queue()
File src/command.h changed (mode: 100644) (index 71ed136..35fb924)
... ... public:
267 267 /* List management */ /* List management */
268 268 song_t add(Songlist *, Song *); song_t add(Songlist *, Song *);
269 269 song_t add(Songlist * source, Songlist * dest); song_t add(Songlist * source, Songlist * dest);
270 int remove(Songlist *, Song *);
270 bool remove(Songlist *, Song *);
271 271 int prune(Songlist *, Songlist *); int prune(Songlist *, Songlist *);
272 272
273 273 /* Play controls */ /* Play controls */
File src/list.cpp changed (mode: 100644) (index 35b8e83..1095527)
... ... song_t Songlist::add(Songlist * list)
481 481
482 482 /* /*
483 483 * Adds a song to the list, either at end or in the middle * Adds a song to the list, either at end or in the middle
484 *
485 * FIXME: vector::erase from the middle of an array is an inefficient operation!
486 *
487 * Returns the zero-indexed position of the added song.
484 488 */ */
485 song_t Songlist::add(Song * song)
489 song_t
490 Songlist::add(Song * song)
486 491 { {
487 492 vector<Song *>::iterator i; vector<Song *>::iterator i;
488 493
489 if (song == NULL)
490 return MPD_SONG_NO_ID;
494 assert(song != NULL);
491 495
492 496 if (song->pos == MPD_SONG_NO_NUM || song->pos == static_cast<song_t>(songs.size())) if (song->pos == MPD_SONG_NO_NUM || song->pos == static_cast<song_t>(songs.size()))
493 497 { {
 
... ... song_t Songlist::add(Song * song)
498 502 } }
499 503 else else
500 504 { {
501 i = songs.begin() + song->pos;
502 if (songs[song->pos]->pos == song->pos) /* FIXME: random crash here? */
503 {
504 if (songs[song->pos]->time != MPD_SONG_NO_TIME)
505 length -= songs[song->pos]->time;
506 i = songs.erase(songs.begin() + song->pos);
505 /* FIXME: random crash here? */
506 if (songs[song->pos]->pos == song->pos) {
507 assert(remove(song) == true);
507 508 } }
509
510 i = songs.begin() + song->pos;
508 511 songs.insert(i, song); songs.insert(i, song);
512
509 513 /* FIXME: filtersongs does not get updated because of ->pos mismatch, but do we need it anyway? */ /* FIXME: filtersongs does not get updated because of ->pos mismatch, but do we need it anyway? */
510 514 } }
511 515
512 if (song->time != MPD_SONG_NO_TIME)
513 {
516 /* FIXME: new function */
517 if (song->time != MPD_SONG_NO_TIME) {
514 518 length += song->time; length += song->time;
515 519 } }
516 520
521 /* FIXME */
517 522 seliter = filtersongs.begin(); seliter = filtersongs.begin();
518 523 rseliter = filtersongs.rbegin(); rseliter = filtersongs.rbegin();
519 524
520 return static_cast<song_t>(songs.size() - 1);
525 return song->pos;
521 526 } }
522 527
523 528 /* /*
524 * Removes a song from the list
529 * Remove a song from the list.
530 *
531 * Returns true on success, false on failure.
525 532 */ */
526 int Songlist::remove(Song * song)
533 bool
534 Songlist::remove(Song * song)
527 535 { {
528 if (!song) return false;
536 assert(song != NULL);
529 537
530 538 selectsong(song, false); selectsong(song, false);
531 539
532 if (song->pos == MPD_SONG_NO_NUM)
533 {
540 if (song->pos == MPD_SONG_NO_NUM) {
534 541 return remove(match(song->file, 0, filtersongs.size() - 1, MATCH_FILE)); return remove(match(song->file, 0, filtersongs.size() - 1, MATCH_FILE));
535 }
536 else return remove(song->pos);
542 }
543
544 return remove(song->pos);
537 545 } }
538 546
539 547 /* /*
540 * Remove song by index
548 * Remove song in position N from the list.
549 *
550 * Returns true on success, false on failure.
541 551 */ */
542 int Songlist::remove(int songpos)
552 bool
553 Songlist::remove(int songpos)
543 554 { {
544 555 vector<Song *>::iterator it; vector<Song *>::iterator it;
545 556 song_t realsongpos; song_t realsongpos;
 
... ... int Songlist::remove(int songpos)
549 560 return false; return false;
550 561 } }
551 562
552 if (songs[songpos]->time != MPD_SONG_NO_TIME)
553 {
563 if (songs[songpos]->time != MPD_SONG_NO_TIME) {
554 564 length -= songs[songpos]->time; length -= songs[songpos]->time;
555 565 } }
556 566
File src/list.h changed (mode: 100644) (index b859fe3..3e8d02d)
... ... public:
164 164
165 165 song_t add(Song *); song_t add(Song *);
166 166 song_t add(Songlist *); song_t add(Songlist *);
167 int remove(Song *);
168 int remove(int);
167 bool remove(Song *);
168 bool remove(int);
169 169 bool move(unsigned int, unsigned int); bool move(unsigned int, unsigned int);
170 170 unsigned int realsize() { return songs.size(); }; unsigned int realsize() { return songs.size(); };
171 171 unsigned int size() { return filtersongs.size(); }; unsigned int size() { return filtersongs.size(); };
Hints

Before first commit, do not forget to setup your git environment:
git config --global user.name "your_name_here"
git config --global user.email "your@email_here"

Clone this repository using HTTP(S):
git clone https://code.reversed.top/user/xaizek/pms

Clone this repository using ssh (do not forget to upload a key first):
git clone ssh://rocketgit@code.reversed.top/user/xaizek/pms

You are allowed to anonymously push to this repository.
This means that your pushed commits will automatically be transformed into a pull request:
... clone the repository ...
... make some changes and some commits ...
git push origin master