From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: trunk r113947: * image.c: Fix animation cache signature memory leak. Date: Mon, 19 Aug 2013 09:56:23 -0700 Organization: UCLA Computer Science Department Message-ID: <52124E37.2080709@cs.ucla.edu> References: <521244F0.3060508@cs.ucla.edu> <521246C4.3070004@cs.ucla.edu> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1376931401 7664 80.91.229.3 (19 Aug 2013 16:56:41 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 19 Aug 2013 16:56:41 +0000 (UTC) Cc: emacs-devel@gnu.org To: Lars Magne Ingebrigtsen Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Aug 19 18:56:43 2013 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1VBSl1-0007VE-0R for ged-emacs-devel@m.gmane.org; Mon, 19 Aug 2013 18:56:43 +0200 Original-Received: from localhost ([::1]:44135 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VBSl0-0002ct-ME for ged-emacs-devel@m.gmane.org; Mon, 19 Aug 2013 12:56:42 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42362) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VBSkq-0002bj-Te for emacs-devel@gnu.org; Mon, 19 Aug 2013 12:56:40 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VBSkj-0001nc-Kd for emacs-devel@gnu.org; Mon, 19 Aug 2013 12:56:32 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:36305) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VBSkj-0001nP-Cd for emacs-devel@gnu.org; Mon, 19 Aug 2013 12:56:25 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 65515A60008; Mon, 19 Aug 2013 09:56:24 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PAuP1M0PBtJv; Mon, 19 Aug 2013 09:56:24 -0700 (PDT) Original-Received: from [192.168.1.9] (pool-71-108-49-126.lsanca.fios.verizon.net [71.108.49.126]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id E4D8EA60007; Mon, 19 Aug 2013 09:56:23 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130803 Thunderbird/17.0.8 In-Reply-To: X-Enigmail-Version: 1.5.2 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 131.179.128.62 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:162887 Archived-At: Lars Magne Ingebrigtsen wrote: > Or am I misreading this completely? No, you got it right. I expect that I ran the wrong executable when I thought I was testing the new code. My code should have looked like the following, I expect. The advantage of doing it this way is that it calls imagemagick_create_cache in just one place, and strcmp in just one place, and refers to 'animation_cache' in just one place, and updates cache->update time in just one place, and overall (to my mind anyway) is easier to understand than the duplications in the current code. But it's only a minor improvement. static struct animation_cache * imagemagick_get_animation_cache (MagickWand *wand) { char *signature = MagickGetImageSignature (wand); struct animation_cache **pcache = &animation_cache; imagemagick_prune_animation_cache (); while (1) { struct animation_cache *cache = *pcache; if (! cache) { *pcache = cache = imagemagick_create_cache (signature); break; } if (strcmp (signature, cache->signature) == 0) { DestroyString (signature); break; } pcache = &cache->next; } cache->update_time = current_emacs_time (); return cache; } By the way, did you know that each animation signature contains 4 KiB of wasted memory? It's ImageMagick code that's wasting it. Dunno if that's worth tuning.