mpdclient: clear the "connection" variable on error

This also adds several "connection==NULL" checks, and thus fixes a
NULL pointer dereference, which could occur when you pressed "tab" too
early, before the connection was established (Debian bug #540415).

diff --git a/NEWS b/NEWS
index de9f1a3aa663fb6dcb3d4c2560380d871de9ba8c..a0f14f8c2de0014141ee140fad87bc835bdb1183 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,7 @@ ncmpc 0.15 - not yet released
 * display song duration in the playlist
 * added the "hardware_cursor" option
 * show plugin error messages on the screen
+* fixed NULL pointer dereference when not yet connected
 * new translation: Hebrew
 
 

diff --git a/src/mpdclient.c b/src/mpdclient.c
index eaef01f65399208d360c208535fa6fb2fc3d5bcd..0897e8607471bba86dbee1bf26dced9e7527b5f2 100644 (file)
--- a/src/mpdclient.c
+++ b/src/mpdclient.c
@@ -40,8 +40,7 @@
 static bool
 MPD_ERROR(const struct mpdclient *client)
 {
-       return client == NULL || client->connection==NULL ||
-               client->connection->error != MPD_ERROR_SUCCESS;
+       return client->connection == NULL;
 }
 
 /* filelist sorting functions */
@@ -124,6 +123,7 @@ static gint
 mpdclient_handle_error(mpdclient_t *c)
 {
        enum mpd_error error = c->connection->error;
+       bool is_fatal = error != MPD_ERROR_ACK;
 
        if (error == MPD_ERROR_SUCCESS)
                return 0;
@@ -137,6 +137,10 @@ mpdclient_handle_error(mpdclient_t *c)
                error = error | (c->connection->errorCode << 8);
 
        error_cb(c, error, c->connection->errorStr);
+
+       if (is_fatal)
+               mpdclient_disconnect(c);
+
        return error;
 }
 
@@ -206,9 +210,16 @@ mpdclient_connect(mpdclient_t *c,
 
        /* connect to MPD */
        c->connection = mpd_newConnection(host, port, _timeout);
-       if( c->connection->error )
-               return error_cb(c, c->connection->error,
-                               c->connection->errorStr);
+       if (c->connection->error) {
+               retval = error_cb(c, c->connection->error,
+                                 c->connection->errorStr);
+               if (retval != 0) {
+                       mpd_closeConnection(c->connection);
+                       c->connection = NULL;
+               }
+
+               return retval;
+       }
 
        /* send password */
        if( password ) {
@@ -275,11 +286,17 @@ mpdclient_cmd_play(mpdclient_t *c, gint idx)
 #ifdef ENABLE_SONG_ID
        struct mpd_song *song = playlist_get_song(c, idx);
 
+       if (MPD_ERROR(c))
+               return -1;
+
        if (song)
                mpd_sendPlayIdCommand(c->connection, song->id);
        else
                mpd_sendPlayIdCommand(c->connection, MPD_PLAY_AT_BEGINNING);
 #else
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendPlayCommand(c->connection, idx);
 #endif
        c->need_update = TRUE;
@@ -289,6 +306,9 @@ mpdclient_cmd_play(mpdclient_t *c, gint idx)
 gint
 mpdclient_cmd_pause(mpdclient_t *c, gint value)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendPauseCommand(c->connection, value);
        return mpdclient_finish_command(c);
 }
@@ -301,6 +321,9 @@ mpdclient_cmd_crop(mpdclient_t *c)
        bool playing;
        int length, current;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendStatusCommand(c->connection);
        status = mpd_getStatus(c->connection);
        error = mpdclient_finish_command(c);
@@ -331,6 +354,9 @@ mpdclient_cmd_crop(mpdclient_t *c)
 gint
 mpdclient_cmd_stop(mpdclient_t *c)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendStopCommand(c->connection);
        return mpdclient_finish_command(c);
 }
@@ -338,6 +364,9 @@ mpdclient_cmd_stop(mpdclient_t *c)
 gint
 mpdclient_cmd_next(mpdclient_t *c)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendNextCommand(c->connection);
        c->need_update = TRUE;
        return mpdclient_finish_command(c);
@@ -346,6 +375,9 @@ mpdclient_cmd_next(mpdclient_t *c)
 gint
 mpdclient_cmd_prev(mpdclient_t *c)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendPrevCommand(c->connection);
        c->need_update = TRUE;
        return mpdclient_finish_command(c);
@@ -354,6 +386,9 @@ mpdclient_cmd_prev(mpdclient_t *c)
 gint
 mpdclient_cmd_seek(mpdclient_t *c, gint id, gint pos)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendSeekIdCommand(c->connection, id, pos);
        return mpdclient_finish_command(c);
 }
@@ -361,6 +396,9 @@ mpdclient_cmd_seek(mpdclient_t *c, gint id, gint pos)
 gint
 mpdclient_cmd_shuffle(mpdclient_t *c)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendShuffleCommand(c->connection);
        c->need_update = TRUE;
        return mpdclient_finish_command(c);
@@ -379,6 +417,9 @@ mpdclient_cmd_clear(mpdclient_t *c)
 {
        gint retval = 0;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendClearCommand(c->connection);
        retval = mpdclient_finish_command(c);
        /* call playlist updated callback */
@@ -390,6 +431,9 @@ mpdclient_cmd_clear(mpdclient_t *c)
 gint
 mpdclient_cmd_repeat(mpdclient_t *c, gint value)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendRepeatCommand(c->connection, value);
        return mpdclient_finish_command(c);
 }
@@ -397,6 +441,9 @@ mpdclient_cmd_repeat(mpdclient_t *c, gint value)
 gint
 mpdclient_cmd_random(mpdclient_t *c, gint value)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendRandomCommand(c->connection, value);
        return mpdclient_finish_command(c);
 }
@@ -404,6 +451,9 @@ mpdclient_cmd_random(mpdclient_t *c, gint value)
 gint
 mpdclient_cmd_single(mpdclient_t *c, gint value)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendSingleCommand(c->connection, value);
        return mpdclient_finish_command(c);
 }
@@ -411,6 +461,9 @@ mpdclient_cmd_single(mpdclient_t *c, gint value)
 gint
 mpdclient_cmd_consume(mpdclient_t *c, gint value)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendConsumeCommand(c->connection, value);
        return mpdclient_finish_command(c);
 }
@@ -418,6 +471,9 @@ mpdclient_cmd_consume(mpdclient_t *c, gint value)
 gint
 mpdclient_cmd_crossfade(mpdclient_t *c, gint value)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendCrossfadeCommand(c->connection, value);
        return mpdclient_finish_command(c);
 }
@@ -427,6 +483,9 @@ mpdclient_cmd_db_update(mpdclient_t *c, const gchar *path)
 {
        gint ret;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendUpdateCommand(c->connection, path ? path : "");
        ret = mpdclient_finish_command(c);
 
@@ -442,12 +501,18 @@ mpdclient_cmd_db_update(mpdclient_t *c, const gchar *path)
 gint
 mpdclient_cmd_volume(mpdclient_t *c, gint value)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendSetvolCommand(c->connection, value);
        return mpdclient_finish_command(c);
 }
 
 gint mpdclient_cmd_volume_up(struct mpdclient *c)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        if (c->status == NULL || c->status->volume == MPD_STATUS_NO_VOLUME)
                return 0;
 
@@ -462,6 +527,9 @@ gint mpdclient_cmd_volume_up(struct mpdclient *c)
 
 gint mpdclient_cmd_volume_down(struct mpdclient *c)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        if (c->status == NULL || c->status->volume == MPD_STATUS_NO_VOLUME)
                return 0;
 
@@ -477,6 +545,9 @@ gint mpdclient_cmd_volume_down(struct mpdclient *c)
 gint
 mpdclient_cmd_add_path(mpdclient_t *c, const gchar *path_utf8)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendAddCommand(c->connection, path_utf8);
        return mpdclient_finish_command(c);
 }
@@ -486,6 +557,9 @@ mpdclient_cmd_add(mpdclient_t *c, const struct mpd_song *song)
 {
        gint retval = 0;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        if( !song || !song->file )
                return -1;
 
@@ -516,6 +590,9 @@ mpdclient_cmd_delete(mpdclient_t *c, gint idx)
        gint retval = 0;
        struct mpd_song *song;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        if (idx < 0 || (guint)idx >= playlist_length(&c->playlist))
                return -1;
 
@@ -561,6 +638,9 @@ mpdclient_cmd_move(mpdclient_t *c, gint old_index, gint new_index)
        gint n;
        struct mpd_song *song1, *song2;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        if (old_index == new_index || new_index < 0 ||
            (guint)new_index >= c->playlist.list->len)
                return -1;
@@ -599,6 +679,9 @@ mpdclient_cmd_save_playlist(mpdclient_t *c, const gchar *filename_utf8)
 {
        gint retval = 0;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendSaveCommand(c->connection, filename_utf8);
        if ((retval = mpdclient_finish_command(c)) == 0)
                mpdclient_browse_callback(c, BROWSE_PLAYLIST_SAVED, NULL);
@@ -608,6 +691,9 @@ mpdclient_cmd_save_playlist(mpdclient_t *c, const gchar *filename_utf8)
 gint
 mpdclient_cmd_load_playlist(mpdclient_t *c, const gchar *filename_utf8)
 {
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendLoadCommand(c->connection, filename_utf8);
        c->need_update = TRUE;
        return mpdclient_finish_command(c);
@@ -618,6 +704,9 @@ mpdclient_cmd_delete_playlist(mpdclient_t *c, const gchar *filename_utf8)
 {
        gint retval = 0;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        mpd_sendRmCommand(c->connection, filename_utf8);
        if ((retval = mpdclient_finish_command(c)) == 0)
                mpdclient_browse_callback(c, BROWSE_PLAYLIST_DELETED, NULL);
@@ -784,6 +873,9 @@ mpdclient_filelist_get(mpdclient_t *c, const gchar *path)
        mpdclient_filelist_t *filelist;
        mpd_InfoEntity *entity;
 
+       if (MPD_ERROR(c))
+               return NULL;
+
        mpd_sendLsInfoCommand(c->connection, path);
        filelist = filelist_new();
        if (path && path[0] && strcmp(path, "/"))
@@ -811,6 +903,9 @@ mpdclient_filelist_search(mpdclient_t *c,
        mpdclient_filelist_t *filelist;
        mpd_InfoEntity *entity;
 
+       if (MPD_ERROR(c))
+               return NULL;
+
        if (exact_match)
                mpd_sendFindCommand(c->connection, table, filter_utf8);
        else
@@ -833,6 +928,9 @@ mpdclient_filelist_add_all(mpdclient_t *c, mpdclient_filelist_t *fl)
 {
        guint i;
 
+       if (MPD_ERROR(c))
+               return -1;
+
        if (filelist_is_empty(fl))
                return 0;
 
@@ -859,6 +957,9 @@ mpdclient_get_artists(mpdclient_t *c)
        gchar *str = NULL;
        GList *list = NULL;
 
+       if (MPD_ERROR(c))
+               return NULL;
+
        mpd_sendListCommand(c->connection, MPD_TABLE_ARTIST, NULL);
        while ((str = mpd_getNextArtist(c->connection)))
                list = g_list_append(list, (gpointer) str);
@@ -875,6 +976,9 @@ mpdclient_get_albums(mpdclient_t *c, const gchar *artist_utf8)
        gchar *str = NULL;
        GList *list = NULL;
 
+       if (MPD_ERROR(c))
+               return NULL;
+
        mpd_sendListCommand(c->connection, MPD_TABLE_ALBUM, artist_utf8);
        while ((str = mpd_getNextAlbum(c->connection)))
                list = g_list_append(list, (gpointer) str);