From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Ken Raeburn Newsgroups: gmane.emacs.devel Subject: Re: using libmagic in Emacs? Date: Sat, 22 Aug 2009 19:13:47 -0400 Message-ID: <5E0028FD-AF0F-4A63-9218-3DCFFFFF0E0E@raeburn.org> References: <83k50xh1m8.fsf@gnu.org> <837hwxgg9w.fsf@gnu.org> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 (Apple Message framework v936) Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1250982859 29031 80.91.229.12 (22 Aug 2009 23:14:19 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 22 Aug 2009 23:14:19 +0000 (UTC) Cc: emacs-devel emacs-devel To: joakim@verona.se Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Aug 23 01:14:13 2009 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1Mezmn-0007Vu-67 for ged-emacs-devel@m.gmane.org; Sun, 23 Aug 2009 01:14:13 +0200 Original-Received: from localhost ([127.0.0.1]:50143 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mezmm-0001zM-Ez for ged-emacs-devel@m.gmane.org; Sat, 22 Aug 2009 19:14:12 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mezmh-0001xt-9o for emacs-devel@gnu.org; Sat, 22 Aug 2009 19:14:07 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mezmc-0001tI-N7 for emacs-devel@gnu.org; Sat, 22 Aug 2009 19:14:07 -0400 Original-Received: from [199.232.76.173] (port=57734 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mezmc-0001t5-Hg for emacs-devel@gnu.org; Sat, 22 Aug 2009 19:14:02 -0400 Original-Received: from splat.raeburn.org ([69.25.196.39]:56124 helo=raeburn.org) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MezmU-0008E7-CT for emacs-devel@gnu.org; Sat, 22 Aug 2009 19:14:02 -0400 Original-Received: from [10.0.0.172] (squish.raeburn.org [10.0.0.172]) by raeburn.org (8.14.3/8.14.1) with ESMTP id n7MNDlRx000675; Sat, 22 Aug 2009 19:13:47 -0400 (EDT) In-Reply-To: X-Mailer: Apple Mail (2.936) X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:114521 Archived-At: On Aug 22, 2009, at 16:18, joakim@verona.se wrote: > Andreas Schwab writes: > joakim@verona.se writes: >> >>> + GCPRO6 (file_description, file_mime, file_encoding, rv, >>> absname, encoded_absname); >> >> That's too much. You only need to protect variables used around >> calls >> that can GC. Arguments to lisp functions are implicitly >> protected. For >> example, there are no function calls during the lifetime of absname. >> And encoded_absname is completely unused. > > It seems to me I only need to protect f, which I would do by GCPRO:ing > absname. Since this is aparently wrong, I will leave it like it is, > since it doesnt hurt to GCPRO too much. (?) If ENCODE_FILE returns a new lisp string object, you need to GCPRO that object, not absname. After the call to ENCODE_FILE, absname is unused, so it won't need protection. In fact, it looks to me like after that point, GC isn't possible, so I'm not sure anything needs GCPRO'tection here. > >>> + libmagic_error: >>> + report_file_error("Libmagic error",Qnil); >>> + if (cookie != NULL) magic_close (cookie); >> >> report_file_error throws, so you leak a resource. > > Fixed I think. No, you're still trying to call magic_close after report_file_error returns, which it won't. > +{ > + CHECK_STRING (filename); > + magic_t cookie=NULL; > + char* f = NULL; CHECK_STRING is executable code, and should be moved down after the variable declarations. > + const char* rvs; > + Lisp_Object file_description; > + Lisp_Object file_mime; > + Lisp_Object file_encoding; > + Lisp_Object rv; > + > + Lisp_Object absname, encoded_absname; > + struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5, gcpro6; > + > + GCPRO6 (file_description, file_mime, file_encoding, rv, absname, > encoded_absname); It seems to be common -- I'm not sure if it's required, but I would conservatively assume so -- for any local variable GCPRO'd to get initialized before any possible call to garbage collection, so the precise(?) garbage collector won't be scanning random stack values and thinking they're lisp objects; if the initialization is unclear, often that means simply assigning Qnil just before or after the GCPRO call. Different GC marking strategies are used on different platforms, so the lack of an obvious problem on one platform doesn't mean the code will work okay on another. Ken