Merge pull request #568 from winterheart/valgrind

Fix discovered memory leaks

Descent3/Mission.cpp

@@ -1914,6 +1914,7 @@ int MissionGetKeywords(const char *mission, char *keywords) {
   }
 
   if (!*parse_keys) {
+    mem_free(parse_keys);
     return MAX_NET_PLAYERS;
   }
   // Break up the mod keywords into an array

Descent3/gamespy.cpp

@@ -209,6 +209,8 @@ int gspy_Init() {
         LOG_INFO.printf("Sending gamespy heartbeats to %s:%d", inet_ntoa(server.sin_addr), htons(server.sin_port));
       }
     }
+
+    cfclose(cfp);
   }
 #endif // #ifndef OEM
   return 1;

libmve/mvelib.cpp

@@ -33,8 +33,8 @@ mve_cb_SetPalette mve_setpalette;
 /*
  * private utility functions
  */
-static short _mve_get_short(unsigned char *data);
-static unsigned short _mve_get_ushort(unsigned char *data);
+static short _mve_get_short(const unsigned char *data);
+static unsigned short _mve_get_ushort(const unsigned char *data);
 
 /*
  * private functions for mvefile
@@ -318,6 +318,8 @@ static void _mvefile_free(MVEFILE *movie) {
  * open the file stream in thie object
  */
 static int _mvefile_open(MVEFILE *file, void *stream) {
+  if (!file)
+    return 0;
   file->stream = stream;
   if (!file->stream)
     return 0;
@@ -343,7 +345,7 @@ static int _mvefile_read_header(MVEFILE *movie) {
   unsigned char buffer[26];
 
   /* check the file is open */
-  if (!movie->stream)
+  if (!movie || !movie->stream)
     return 0;
 
   /* check the file is long enough */
@@ -351,7 +353,7 @@ static int _mvefile_read_header(MVEFILE *movie) {
     return 0;
 
   /* check the signature */
-  if (memcmp(buffer, MVE_HEADER, 20))
+  if (memcmp(buffer, MVE_HEADER, 20) != 0)
     return 0;
 
   /* check the hard-coded constants */
@@ -366,6 +368,9 @@ static int _mvefile_read_header(MVEFILE *movie) {
 }
 
 static void _mvefile_set_buffer_size(MVEFILE *movie, int buf_size) {
+  if (!movie)
+    return;
+
   unsigned char *new_buffer;
   int new_len;
 
@@ -384,7 +389,7 @@ static void _mvefile_set_buffer_size(MVEFILE *movie, int buf_size) {
   /* free old buffer */
   if (movie->cur_chunk) {
     mve_free(movie->cur_chunk);
-    movie->cur_chunk = 0;
+    movie->cur_chunk = nullptr;
   }
 
   /* install new buffer */
@@ -397,7 +402,7 @@ static int _mvefile_fetch_next_chunk(MVEFILE *movie) {
   unsigned short length;
 
   /* fail if not open */
-  if (!movie->stream)
+  if (!movie || !movie->stream)
     return 0;
 
   /* fail if we can't read the next segment descriptor */
@@ -419,13 +424,13 @@ static int _mvefile_fetch_next_chunk(MVEFILE *movie) {
   return 1;
 }
 
-static short _mve_get_short(unsigned char *data) {
+static short _mve_get_short(const unsigned char *data) {
   short value;
   value = data[0] | (data[1] << 8);
   return value;
 }
 
-static unsigned short _mve_get_ushort(unsigned char *data) {
+static unsigned short _mve_get_ushort(const unsigned char *data) {
   unsigned short value;
   value = data[0] | (data[1] << 8);
   return value;
@@ -440,7 +445,7 @@ static MVESTREAM *_mvestream_alloc() {
   /* allocate and zero-initialize everything */
   movie = (MVESTREAM *)mve_alloc(sizeof(MVESTREAM));
   movie->movie = nullptr;
-  movie->context = 0;
+  movie->context = nullptr;
   memset(movie->handlers, 0, sizeof(movie->handlers));
 
   return movie;
@@ -469,6 +474,8 @@ static void _mvestream_free(MVESTREAM *movie) {
  * open an MVESTREAM object
  */
 static int _mvestream_open(MVESTREAM *movie, void *stream) {
+  if (!movie)
+    return 0;
   movie->movie = mvefile_open(stream);
 
   return (movie->movie == nullptr) ? 0 : 1;

libmve/mvelib.h

@@ -23,7 +23,7 @@
 
 /* callback for reading stream */
 typedef unsigned int (*mve_cb_Read)(void *stream, void *buffer, unsigned int count);
-/* callback for memore allocating */
+/* callback for memory allocating */
 typedef void *(*mve_cb_Alloc)(unsigned int size);
 /* callback for memory freeing */
 typedef void (*mve_cb_Free)(void *ptr);
@@ -76,7 +76,7 @@ void MVE_rmEndMovie(MVESTREAM *mve);
 void MVE_getVideoSpec(MVE_videoSpec *vSpec);
 
 // Initialize MVE sound. Set `enable` to false if sound should not be enabled.
-void MVE_sndInit(const bool enable);
+void MVE_sndInit(bool enable);
 
 void MVE_ioCallbacks(mve_cb_Read io_read);
 void MVE_memCallbacks(mve_cb_Alloc mem_alloc, mve_cb_Free mem_free);

libmve/mveplay.cpp

@@ -470,7 +470,7 @@ void MVE_rmEndMovie(MVESTREAM *mve) {
 
 void MVE_rmHoldMovie() { timer_started = 0; }
 
-void MVE_sndInit(const bool enable) {
+void MVE_sndInit(bool enable) {
 #ifdef AUDIO
   if (enable) {
     mve_audio_enabled = 1;

manage/generic.cpp

@@ -1142,7 +1142,7 @@ int mng_ReadNewGenericPage(CFILE *infile, mngs_generic_page *genericpage) {
     char tempbuf[1024];
 
     cf_ReadString(tempbuf, 1024, infile);
-    int slen = strlen(tempbuf) + 1;
+    size_t slen = strlen(tempbuf) + 1;
 
     genericpage->objinfo_struct.description = (char *)mem_malloc(slen);
     ASSERT(genericpage->objinfo_struct.description);
@@ -1927,10 +1927,7 @@ int mng_AssignGenericPageToObjInfo(mngs_generic_page *genericpage, int n, CFILE
   if (objinfopointer->ai_info)
     memcpy(objinfopointer->ai_info, &genericpage->ai_info, sizeof(genericpage->ai_info));
 
-  objinfopointer->multi_allowed = 1;
-
-  // since the description pointer was just copied over, no need to malloc mem, copy and then free old, just move ptr
-  genericpage->objinfo_struct.description = NULL;
+  objinfopointer->multi_allowed = true;
 
   strcpy(objinfopointer->icon_name, genericpage->objinfo_struct.icon_name);
   // First see if our image differs from the one on the net
@@ -2195,7 +2192,7 @@ void mng_AssignObjInfoToGenericPage(int n, mngs_generic_page *genericpage) {
 // then calls SetAndLoadgeneric to actually load in any images/models associated
 // with it
 void mng_LoadNetGenericPage(CFILE *infile, bool overlay) {
-  mngs_generic_page genericpage;
+  mngs_generic_page genericpage{};
   memset(&genericpage, 0, sizeof(mngs_generic_page));
 
   if (mng_ReadNewGenericPage(infile, &genericpage)) {
@@ -2206,11 +2203,21 @@ void mng_LoadNetGenericPage(CFILE *infile, bool overlay) {
         mng_FreePagetypePrimitives(PAGETYPE_GENERIC, genericpage.objinfo_struct.name, 0);
         mng_AssignGenericPageToObjInfo(&genericpage, n);
       }
+      // Free allocated memory
+      if (genericpage.objinfo_struct.description) {
+        mem_free(genericpage.objinfo_struct.description);
+        genericpage.objinfo_struct.description = nullptr;
+      }
       return; // A weapon has already loaded this generic
     }
 
     int ret = mng_SetAndLoadGeneric(&genericpage, infile);
     ASSERT(ret >= 0);
+    // Free allocated memory
+    if (genericpage.objinfo_struct.description) {
+      mem_free(genericpage.objinfo_struct.description);
+      genericpage.objinfo_struct.description = nullptr;
+    }
   } else
     LOG_ERROR.printf("Could not load genericpage named %s!", genericpage.objinfo_struct.name);
 }