From d0139df5b60d235363e78438316e255ac378c884 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 19 Aug 2013 00:01:37 -0700 Subject: [PATCH] * 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. --- src/ChangeLog | 11 +++++++++ src/image.c | 62 +++++++++++++++++++++++---------------------------- 2 files changed, 39 insertions(+), 34 deletions(-) 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 + * 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 (); -- 2.39.2