Author: Pedro Lucas Porcellis <porcellis@eletrotupi.com>
queue: patch a fuckton of crash bugs Fix off-by-one, guard stitle/sartist against NULL before strlen/strcpy free old queue before re-allocation to prevent memory leaks Also yeet dead return 0 and verbose comment block
src/queue.c | 56 +++++++++++++++++++++---------------------------------
diff --git a/src/queue.c b/src/queue.c index af8f25da8aed87c3dd87a669e9b2a273754dcbbe..80d967e46e17e28e480eafc69dddc9b853f3f941 100644 --- a/src/queue.c +++ b/src/queue.c @@ -1,4 +1,5 @@ #include <stdio.h> +#include <stdlib.h> #include <assert.h> #include <stdbool.h> #include "seamus.h" @@ -15,62 +16,49 @@ struct mpd_entity *entity; int index = 0; int max_items = 0; - // XXX: The logic here is: We check the amount of items queued. - // Then we have the info o how many items we can fit into the - // screen. The decision we need to make here is: if the amount - // of items queued is larger than what we can display on the - // screen, alloc up to the number of items on screen (we can't - // show more than that anyway). If the number of items queued is - // lesser than the amount of rows we have available, only alloc - // up to the number of items on queue. - if (seamus->status->length > max_count) { - seamus->queue = calloc(max_count, sizeof(struct seamus_song)); max_items = max_count; } else { - seamus->queue = calloc(seamus->status->length, sizeof(struct seamus_song)); max_items = seamus->status->length; } + free(seamus->queue); + seamus->queue = calloc(max_items, sizeof(struct seamus_song)); seamus->queue_size = max_items; - while (index <= max_items) { + while (index < max_items) { entity = mpd_recv_entity(seamus->conn); if (entity == NULL) { - // TODO: Discover if the queue have ended, or we - // have received an error here break; + } - return 0; - } else { - enum mpd_entity_type type = mpd_entity_get_type(entity); + enum mpd_entity_type type = mpd_entity_get_type(entity); - if (type == MPD_ENTITY_TYPE_SONG) { - const struct mpd_song *song = mpd_entity_get_song(entity); + if (type == MPD_ENTITY_TYPE_SONG) { + const struct mpd_song *song = mpd_entity_get_song(entity); - const char *stitle = mpd_song_get_tag(song, MPD_TAG_TITLE, 0); - const char *sartist = mpd_song_get_tag(song, MPD_TAG_ALBUM_ARTIST, 0); + const char *stitle = mpd_song_get_tag(song, MPD_TAG_TITLE, 0); + const char *sartist = mpd_song_get_tag(song, MPD_TAG_ALBUM_ARTIST, 0); - struct seamus_song *new = &seamus->queue[index]; - memset(new, 0, sizeof(*new)); + if (stitle == NULL) stitle = ""; + if (sartist == NULL) sartist = ""; - char *title = malloc(sizeof(char) * strlen(stitle) + 1); - strcpy(title, stitle); + struct seamus_song *new = &seamus->queue[index]; + memset(new, 0, sizeof(*new)); - char *artist = malloc(sizeof(char) * strlen(sartist) + 1); - strcpy(artist, sartist); + char *title = malloc(strlen(stitle) + 1); + strcpy(title, stitle); - new->song_id = mpd_song_get_id(song); - new->title = title; - new->artist = artist; - } + char *artist = malloc(strlen(sartist) + 1); + strcpy(artist, sartist); - // When freeing the entity, it'll automatically - // free the song for us - mpd_entity_free(entity); + new->song_id = mpd_song_get_id(song); + new->title = title; + new->artist = artist; } + mpd_entity_free(entity); index++; }