From: Mike Brady <mikebrady@eircom.net>
Date: Sun, 22 Jul 2018 15:12:52 +0000 (+0100)
Subject: Fix a few memory leaks.
X-Git-Tag: 3.3RC0~286^2~14
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5f750ba9b727c384b992711a95a32fe0a48dcecd;p=thirdparty%2Fshairport-sync.git

Fix a few memory leaks.
---

diff --git a/audio_alsa.c b/audio_alsa.c
index 1a61858e..3f00fddd 100644
--- a/audio_alsa.c
+++ b/audio_alsa.c
@@ -908,6 +908,7 @@ static void flush(void) {
   debug_mutex_lock(&alsa_mutex, 10000, 1);
   int derr;
   do_mute(1);
+  
   if (alsa_handle) {
 
     if ((derr = snd_pcm_drop(alsa_handle)))
diff --git a/common.c b/common.c
index 20f0b5b9..0bfdebb8 100644
--- a/common.c
+++ b/common.c
@@ -399,7 +399,8 @@ uint8_t *base64_dec(char *input, int *outlen) {
 
   nread = BIO_read(b64, buf, bufsize);
 
-  BIO_free_all(bmem);
+  // BIO_free_all(bmem);
+  BIO_free_all(b64);
 
   *outlen = nread;
   return buf;
diff --git a/dacp.c b/dacp.c
index 667ac19c..80ccb2ed 100644
--- a/dacp.c
+++ b/dacp.c
@@ -122,6 +122,35 @@ static pthread_mutex_t dacp_conversation_lock;
 static pthread_mutex_t dacp_server_information_lock;
 static pthread_cond_t dacp_server_information_cv = PTHREAD_COND_INITIALIZER;
 
+void addrinfo_cleanup(void *arg) {
+  debug(1, "addrinfo cleanup called.");
+  struct addrinfo **info = (struct addrinfo **)arg;
+  freeaddrinfo(*info);
+}
+
+void mutex_lock_cleanup(void *arg) {
+  debug(1, "mutex lock cleanup called.");
+  pthread_mutex_t *m = (pthread_mutex_t *)arg;
+  pthread_mutex_unlock(m);
+}
+
+void connect_cleanup(void *arg) {
+  debug(1, "connect cleanup called.");
+  int *fd = (int *)arg;
+  close(*fd);
+}
+
+void http_cleanup(void *arg) {
+  debug(1, "http cleanup called.");
+  struct http_roundtripper *rt = (struct http_roundtripper *)arg;
+  http_free(rt);
+}
+
+void malloc_cleanup(void *arg) {
+  debug(1, "malloc cleanup called.");
+  free(arg);
+}
+
 int dacp_send_command(const char *command, char **body, ssize_t *bodysize) {
 
   // will malloc space for the body or set it to NULL -- the caller should free it.
@@ -166,12 +195,13 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) {
     // debug(1,"Error %d \"%s\" at getaddrinfo.",ires,gai_strerror(ires));
     response.code = 498; // Bad Address information for the DACP server
   } else {
-
+    pthread_cleanup_push(addrinfo_cleanup, (void *)&res);
     // only do this one at a time -- not sure it is necessary, but better safe than sorry
 
     int mutex_reply = sps_pthread_mutex_timedlock(&dacp_conversation_lock, 2000000, command, 1);
     // int mutex_reply = pthread_mutex_lock(&dacp_conversation_lock);
     if (mutex_reply == 0) {
+      pthread_cleanup_push(mutex_lock_cleanup, (void *)&dacp_conversation_lock);
       // debug(1,"dacp_conversation_lock acquired for command \"%s\".",command);
 
       // make a socket:
@@ -195,6 +225,7 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) {
           debug(3, "DACP connect failed with errno %d.", errno);
           response.code = 496; // Can't connect to the DACP server
         } else {
+          pthread_cleanup_push(connect_cleanup, (void *)&sockfd);
           // debug(1,"DACP connect succeeded.");
 
           snprintf(message, sizeof(message),
@@ -213,9 +244,11 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) {
 
             response.body = malloc(2048); // it can resize this if necessary
             response.malloced_size = 2048;
+            pthread_cleanup_push(malloc_cleanup, response.body);
 
             struct http_roundtripper rt;
             http_init(&rt, responseFuncs, &response);
+            pthread_cleanup_push(http_cleanup, &rt);
 
             int needmore = 1;
             int looperror = 0;
@@ -254,13 +287,18 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) {
               response.size = 0;
             }
             // debug(1,"Size of response body is %d",response.size);
-            http_free(&rt);
+            pthread_cleanup_pop(1); // this should call http_cleanup
+            // http_free(&rt);
+            pthread_cleanup_pop(
+                0); // this should *not* free the malloced buffer -- just pop the malloc cleanup
           }
+          pthread_cleanup_pop(1); // this should close the socket
+                                  // close(sockfd);
+                                  // debug(1,"DACP socket closed.");
         }
-        close(sockfd);
-        // debug(1,"DACP socket closed.");
       }
-      pthread_mutex_unlock(&dacp_conversation_lock);
+      pthread_cleanup_pop(1); // this should unlock the dacp_conversation_lock);
+      // pthread_mutex_unlock(&dacp_conversation_lock);
       // debug(1,"Sent command\"%s\" with a response body of size %d.",command,response.size);
       // debug(1,"dacp_conversation_lock released.");
     } else {
@@ -270,6 +308,8 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) {
             command);
       response.code = 494; // This client is already busy
     }
+    pthread_cleanup_pop(1); // this should free the addrinfo
+    // freeaddrinfo(res);
   }
   *body = response.body;
   *bodysize = response.size;
@@ -930,12 +970,13 @@ int dacp_get_speaker_list(dacp_spkr_stuff *speaker_info, int max_size_of_array,
             sp -= item_size;
             le -= 8;
             speaker_index++;
-            if (speaker_index == max_size_of_array)
+            if (speaker_index == max_size_of_array) {
               return 413; // Payload Too Large -- too many speakers
+            }
             speaker_info[speaker_index].active = 0;
             speaker_info[speaker_index].speaker_number = 0;
             speaker_info[speaker_index].volume = 0;
-            speaker_info[speaker_index].name = NULL;
+            speaker_info[speaker_index].name[0] = '\0';
           } else {
             le -= item_size + 8;
             char *t;
@@ -945,8 +986,10 @@ int dacp_get_speaker_list(dacp_spkr_stuff *speaker_info, int max_size_of_array,
             switch (type) {
             case 'minm':
               t = sp - item_size;
-              speaker_info[speaker_index].name = strndup(t, item_size);
-              // debug(1," \"%s\"",speaker_info[speaker_index].name);
+              strncpy((char *)&speaker_info[speaker_index].name, t,
+                      sizeof(speaker_info[speaker_index].name));
+              speaker_info[speaker_index].name[sizeof(speaker_info[speaker_index].name) - 1] =
+                  '\0'; // just in case
               break;
             case 'cmvo':
               t = sp - item_size;
diff --git a/dacp.h b/dacp.h
index 5379d069..ce1e1278 100644
--- a/dacp.h
+++ b/dacp.h
@@ -10,7 +10,7 @@ typedef struct dacp_speaker_stuff {
   int64_t speaker_number;
   int active;
   int32_t volume;
-  char *name; // this is really just for debugging
+  char name[128]; // this is really just for debugging
 } dacp_spkr_stuff;
 
 void dacp_monitor_start();
diff --git a/mdns_avahi.c b/mdns_avahi.c
index c9b8ae00..d9eb4b02 100644
--- a/mdns_avahi.c
+++ b/mdns_avahi.c
@@ -385,8 +385,8 @@ static int avahi_register(char *srvname, int srvport) {
 static void avahi_unregister(void) {
   debug(1, "avahi: avahi_unregister.");
   if (tpoll) {
-    //    debug(1, "avahi: stop the threaded poll.");
-    //    avahi_threaded_poll_stop(tpoll);
+    debug(1, "avahi: stop the threaded poll.");
+    avahi_threaded_poll_stop(tpoll);
 
     if (client) {
       debug(1, "avahi: free the client.");
diff --git a/shairport.c b/shairport.c
index b4136314..3c843845 100644
--- a/shairport.c
+++ b/shairport.c
@@ -1173,7 +1173,10 @@ void main_cleanup_handler(__attribute__((unused)) void *arg) {
 #ifdef CONFIG_METADATA
   metadata_stop(); // close down the metadata pipe
 #endif
-
+  if (config.output->deinit) {
+    debug(1,"Deinitialise the audio backend.");
+    config.output->deinit();
+  }
   daemon_log(LOG_NOTICE, "Unexpected exit...");
   daemon_retval_send(0);
   daemon_pid_file_remove();
@@ -1449,7 +1452,6 @@ int main(int argc, char **argv) {
   // make sure the program can create files that group and world can read
   umask(S_IWGRP | S_IWOTH);
 
-  pthread_cleanup_push(main_cleanup_handler, NULL);
 
   config.output = audio_get_output(config.output_name);
   if (!config.output) {
@@ -1458,6 +1460,8 @@ int main(int argc, char **argv) {
   }
   config.output->init(argc - audio_arg, argv + audio_arg);
 
+  pthread_cleanup_push(main_cleanup_handler, NULL);
+
   // daemon_log(LOG_NOTICE, "startup");
 
   switch (endianness) {