Author: Pedro Lucas Porcellis <porcellis@eletrotupi.com>
Rewrite generate_description and clean up fetch_current_status - Replace 4-allocation string building in generate_description with a single snprintf directly to status->description - Guard title/artist against NULL before formatting - Move mpd_song_free ownership to caller (fetch_current_status) - Add 'Stopped' fallback description when no song is playing - Remove redundant description = NULL after calloc - Remove dead commented-out code
src/status.c | 78 +++++++++++++----------------------------------------
diff --git a/src/status.c b/src/status.c index 03ac64aaa12026d51218214ce81fa1dd114ae332..4dfdf40d699302b310c515d40e4b18efdb773678 100644 --- a/src/status.c +++ b/src/status.c @@ -1,5 +1,6 @@ #include <assert.h> #include <stdio.h> +#include <stdlib.h> #include <string.h> #include "seamus.h" #include "status.h" @@ -18,13 +19,8 @@ return 1; } - // Initialize status if not already done if (s->status == NULL) { - struct seamus_status *s_status = calloc(1, sizeof(struct seamus_status)); - if (s_status) { - s_status->description = NULL; - s->status = s_status; - } + s->status = calloc(1, sizeof(struct seamus_status)); } s->status->repeat = mpd_status_get_repeat(status); @@ -41,10 +37,15 @@ struct mpd_song *song = mpd_run_current_song(s->conn); if (song != NULL) { int desc = generate_description(s->status, song); + mpd_song_free(song); if (desc != 0) { log_debug("Something went off, when generate description"); } + } else { + free(s->status->description); + s->status->description = malloc(sizeof("Stopped")); + sprintf(s->status->description, "%s", "Stopped"); } mpd_status_free(status); @@ -56,66 +57,25 @@ int generate_description(struct seamus_status *status, struct mpd_song *song) { - const char *title = mpd_song_get_tag(song, MPD_TAG_TITLE, 0); const char *artist = mpd_song_get_tag(song, MPD_TAG_ARTIST, 0); - char *elapsed_time = (char*) malloc(13 * sizeof(char)); - - sprintf(elapsed_time, "%3i:%02i", - status->elapsed_time / 60, - status->elapsed_time % 60); - - char *total_time = (char*) malloc(13 * sizeof(char)); - - sprintf(total_time, "%i:%02i", - status->total_time / 60, - status->total_time % 60); - - // TODO: This looks kind of bad. Probably have some - // less stupid way to do it - size_t strsz = ( - sizeof(char) * ( - strlen(artist) + - strlen(title) + - strlen(elapsed_time) + - strlen(total_time) + - 20 - ) - ); - - char *str = malloc(strsz); + if (title == NULL) title = ""; + if (artist == NULL) artist = ""; - if (status->description != NULL) { - free(status->description); - } + unsigned em = status->elapsed_time / 60; + unsigned es = status->elapsed_time % 60; + unsigned tm = status->total_time / 60; + unsigned ts = status->total_time % 60; - status->description = malloc(strsz); + int n = snprintf(NULL, 0, "%s - %s: %u:%02u/%u:%02u", + artist, title, em, es, tm, ts); - sprintf(str, "%s - %s: %s/%s", - artist, - title, - elapsed_time, - total_time - ); + free(status->description); + status->description = malloc(n + 1); - strcpy(status->description, str); - - free(total_time); - free(elapsed_time); - free(str); - mpd_song_free(song); + sprintf(status->description, "%s - %s: %u:%02u/%u:%02u", + artist, title, em, es, tm, ts); return 0; -//} else { -// const char *stat = "Stopped"; -// -// // This works like a charm, albeit it's ugly: -// status->description = malloc(sizeof(char) * strlen(stat)); -// strcpy(status->description, stat); -// -// return 0; -//} - -//return 1; }