From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 19 Aug 2013 07:01:37 +0000 (-0700)
Subject: * image.c: Fix animation cache signature memory leak.
X-Git-Tag: emacs-24.3.90~173^2^2~42^2~45^2~387^2~1686^2~176
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=d0139df5b60d235363e78438316e255ac378c884;p=emacs.git

* image.c: Fix animation cache signature memory leak.

Fix some other minor performance problems while we're at it.
(imagemagick_create_cache): Clear just the members that
need clearing.  Don't set update_time, as caller does that now.
(imagemagick_prune_animation_cache, imagemagick_get_animation_cache):
Simplify by using pointer-to-pointer instead of a prev pointer.
(imagemagick_prune_animation_cache): Use make_emacs_time rather
than EMACS_TIME_FROM_DOUBLE, and DestroyString rather than free.
(imagemagick_get_animation_cache): Don't xstrdup the image signature;
it's already a copy.  Free the signature probe unless it's cached.
---

diff --git a/src/ChangeLog b/src/ChangeLog
index 0203ce636ef..c5a6f4d19c3 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,16 @@
 2013-08-19  Paul Eggert  <eggert@cs.ucla.edu>
 
+	* image.c: Fix animation cache signature memory leak.
+	Fix some other minor performance problems while we're at it.
+	(imagemagick_create_cache): Clear just the members that
+	need clearing.  Don't set update_time, as caller does that now.
+	(imagemagick_prune_animation_cache, imagemagick_get_animation_cache):
+	Simplify by using pointer-to-pointer instead of a prev pointer.
+	(imagemagick_prune_animation_cache): Use make_emacs_time rather
+	than EMACS_TIME_FROM_DOUBLE, and DestroyString rather than free.
+	(imagemagick_get_animation_cache): Don't xstrdup the image signature;
+	it's already a copy.  Free the signature probe unless it's cached.
+
 	* process.c (handle_child_signal): Fix crash; deleted pid (Bug#15106).
 	This was introduced by my 2013-08-12 fix for Bug#15035.
 
diff --git a/src/image.c b/src/image.c
index a8210e2414a..68d78fc9eef 100644
--- a/src/image.c
+++ b/src/image.c
@@ -7890,9 +7890,11 @@ static struct animation_cache *animation_cache = NULL;
 static struct animation_cache *
 imagemagick_create_cache (char *signature)
 {
-  struct animation_cache *cache = xzalloc (sizeof *cache);
+  struct animation_cache *cache = xmalloc (sizeof *cache);
   cache->signature = signature;
-  cache->update_time = current_emacs_time ();
+  cache->wand = 0;
+  cache->index = 0;
+  cache->next = 0;
   return cache;
 }
 
@@ -7900,30 +7902,22 @@ imagemagick_create_cache (char *signature)
 static void
 imagemagick_prune_animation_cache (void)
 {
-  struct animation_cache *cache = animation_cache;
-  struct animation_cache *prev = NULL;
+  struct animation_cache **pcache = &animation_cache;
   EMACS_TIME old = sub_emacs_time (current_emacs_time (),
-				   EMACS_TIME_FROM_DOUBLE (60));
+				   make_emacs_time (60, 0));
 
-  while (cache)
+  while (*pcache)
     {
-      if (EMACS_TIME_LT (cache->update_time, old))
+      struct animation_cache *cache = *pcache;
+      if (EMACS_TIME_LE (old, cache->update_time))
+	pcache = &cache->next;
+      else
 	{
-	  struct animation_cache *this_cache = cache;
-	  free (cache->signature);
+	  DestroyString (cache->signature);
 	  if (cache->wand)
 	    DestroyMagickWand (cache->wand);
-	  if (prev)
-	    prev->next = cache->next;
-	  else
-	    animation_cache = cache->next;
-	  cache = cache->next;
-	  free (this_cache);
-	}
-      else
-	{
-	  prev = cache;
-	  cache = cache->next;
+	  *pcache = cache->next;
+	  xfree (cache);
 	}
     }
 }
@@ -7931,26 +7925,26 @@ imagemagick_prune_animation_cache (void)
 static struct animation_cache *
 imagemagick_get_animation_cache (MagickWand *wand)
 {
-  char *signature = xstrdup (MagickGetImageSignature (wand));
+  char *signature = MagickGetImageSignature (wand);
   struct animation_cache *cache;
+  struct animation_cache **pcache = &animation_cache;
 
   imagemagick_prune_animation_cache ();
   cache = animation_cache;
 
-  if (! cache)
-    {
-      animation_cache = imagemagick_create_cache (signature);
-      return animation_cache;
-    }
-
-  while (strcmp(signature, cache->signature) &&
-	 cache->next)
-    cache = cache->next;
-
-  if (strcmp(signature, cache->signature))
+  for (pcache = &animation_cache; *pcache; pcache = &cache->next)
     {
-      cache->next = imagemagick_create_cache (signature);
-      return cache->next;
+      cache = *pcache;
+      if (! cache)
+	{
+	  *pcache = cache = imagemagick_create_cache (signature);
+	  break;
+	}
+      if (strcmp (signature, cache->signature) == 0)
+	{
+	  DestroyString (signature);
+	  break;
+	}
     }
 
   cache->update_time = current_emacs_time ();