From 95895823f75108cbfed2697498e4e097f493b236 Mon Sep 17 00:00:00 2001
From: Tavian Barnes <tavianator@gmail.com>
Date: Mon, 19 Oct 2009 00:49:29 +0000
Subject: Fix some memory leaks.

dmnsn_delete_pigment() was not using the free_fn, and kD splay trees were not
being deleted after raytracing finished.
---
 libdimension/kD_splay_tree.c | 26 ++++++++++++++++----------
 libdimension/pigments.c      |  1 +
 libdimension/raytrace.c      |  8 ++++++--
 libdimension/texture.c       |  9 +++++++--
 4 files changed, 30 insertions(+), 14 deletions(-)

(limited to 'libdimension')

diff --git a/libdimension/kD_splay_tree.c b/libdimension/kD_splay_tree.c
index de70edf..a661602 100644
--- a/libdimension/kD_splay_tree.c
+++ b/libdimension/kD_splay_tree.c
@@ -29,10 +29,11 @@ dmnsn_kD_splay_tree *
 dmnsn_new_kD_splay_tree()
 {
   dmnsn_kD_splay_tree *tree = malloc(sizeof(dmnsn_kD_splay_tree));
-  if (!tree) {
+  if (tree) {
+    tree->root = NULL;
+  } else {
     dmnsn_error(DMNSN_SEVERITY_HIGH, "kD splay tree allocation failed.");
   }
-  tree->root = NULL;
   return tree;
 }
 
@@ -67,20 +68,25 @@ dmnsn_kD_splay_copy(dmnsn_kD_splay_tree *tree)
 
 /* Recursively free a kD splay tree */
 void
-dmnsn_delete_kD_splay_tree(dmnsn_kD_splay_tree *tree)
+dmnsn_delete_kD_splay_tree_recursive(dmnsn_kD_splay_node *node)
 {
-  dmnsn_kD_splay_node *node = tree->root;
   if (node) {
-    tree->root = node->contains;
-    dmnsn_delete_kD_splay_tree(tree);
-
-    tree->root = node->container;
-    dmnsn_delete_kD_splay_tree(tree);
-
+    dmnsn_delete_kD_splay_tree_recursive(node->contains);
+    dmnsn_delete_kD_splay_tree_recursive(node->container);
     dmnsn_delete_kD_splay_node(node);
   }
 }
 
+/* Free a kD splay tree */
+void
+dmnsn_delete_kD_splay_tree(dmnsn_kD_splay_tree *tree)
+{
+  if (tree) {
+    dmnsn_delete_kD_splay_tree_recursive(tree->root);
+    free(tree);
+  }
+}
+
 /* Return whether node1 contains node2 */
 static int dmnsn_box_contains(dmnsn_vector p,
                               dmnsn_vector min, dmnsn_vector max);
diff --git a/libdimension/pigments.c b/libdimension/pigments.c
index 22f75e8..adf44c4 100644
--- a/libdimension/pigments.c
+++ b/libdimension/pigments.c
@@ -40,6 +40,7 @@ dmnsn_new_solid_pigment(dmnsn_color color)
     *solid = color;
 
     pigment->pigment_fn = &dmnsn_solid_pigment_fn;
+    pigment->free_fn    = &free;
     pigment->ptr        = solid;
   }
 
diff --git a/libdimension/raytrace.c b/libdimension/raytrace.c
index 3e67c4b..bb88940 100644
--- a/libdimension/raytrace.c
+++ b/libdimension/raytrace.c
@@ -140,7 +140,7 @@ dmnsn_raytrace_scene_multithread(dmnsn_raytrace_payload *payload)
     payloads[i].threads = nthreads;
     if (i > 0) {
       payloads[i].kD_splay_tree =
-        dmnsn_kD_splay_copy(payloads[i].kD_splay_tree);
+        dmnsn_kD_splay_copy(payloads[0].kD_splay_tree);
     }
 
     if (pthread_create(&threads[i], NULL,
@@ -148,11 +148,14 @@ dmnsn_raytrace_scene_multithread(dmnsn_raytrace_payload *payload)
                        &payloads[i]) != 0)
     {
       for (j = 0; j < i; ++j) {
-        if (pthread_join(threads[i], &ptr)) {
+        if (pthread_join(threads[j], &ptr)) {
           dmnsn_error(DMNSN_SEVERITY_MEDIUM,
                       "Couldn't join worker thread in failed raytrace engine"
                       " initialization.");
         } else {
+          /* Only free on a successful join - otherwise we might free a pointer
+             out from under a running thread */
+          dmnsn_delete_kD_splay_tree(payloads[j].kD_splay_tree);
           free(ptr);
         }
       }
@@ -170,6 +173,7 @@ dmnsn_raytrace_scene_multithread(dmnsn_raytrace_payload *payload)
       if (retval == 0) {
         retval = *(int *)ptr;
       }
+      dmnsn_delete_kD_splay_tree(payloads[i].kD_splay_tree);
       free(ptr);
     }
   }
diff --git a/libdimension/texture.c b/libdimension/texture.c
index a5ebd90..3867628 100644
--- a/libdimension/texture.c
+++ b/libdimension/texture.c
@@ -32,11 +32,16 @@ dmnsn_new_pigment()
   return pigment;
 }
 
-/* Free a dummy pigment */
+/* Free a pigment */
 void
 dmnsn_delete_pigment(dmnsn_pigment *pigment)
 {
-  free(pigment);
+  if (pigment) {
+    if (pigment->free_fn) {
+      (*pigment->free_fn)(pigment->ptr);
+    }
+    free(pigment);
+  }
 }
 
 /* Allocate a dummy texture */
-- 
cgit v1.2.3