[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: using libmagic in Emacs?

From: Ken Raeburn
Subject: Re: using libmagic in Emacs?
Date: Sat, 22 Aug 2009 19:13:47 -0400

On Aug 22, 2009, at 16:18, address@hidden wrote:
Andreas Schwab <address@hidden> writes:
address@hidden 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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]