Re: using libmagic in Emacs?

[joakim]

Ken Raeburn <address@hidden> writes:

> 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.
>

I seem to be having trouble with GCPRO. Now I've marked the places I
believe might gc in the code.

>>> 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.

Maybe I sent the wrong patch revision last time, better now then?

> CHECK_STRING is executable code, and should be moved down after the
> variable declarations.

same here.


> 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
-- 
Joakim Verona

diff --git a/configure.in b/configure.in
index f4096db..49a3f15 100644
--- a/configure.in
+++ b/configure.in
@@ -137,6 +137,8 @@ OPTION_DEFAULT_ON([xft],[don't use XFT for anti aliased 
fonts])
 OPTION_DEFAULT_ON([libotf],[don't use libotf for OpenType font support])
 OPTION_DEFAULT_ON([m17n-flt],[don't use m17n-flt for text shaping])
 
+OPTION_DEFAULT_ON([libmagic],[don't compile with libmagic support])
+
 OPTION_DEFAULT_ON([toolkit-scroll-bars],[don't use Motif or Xaw3d scroll bars])
 OPTION_DEFAULT_ON([xaw3d],[don't use Xaw3d])
 OPTION_DEFAULT_ON([xim],[don't use X11 XIM])
@@ -2223,6 +2225,19 @@ if test x"$ac_cv_func_alloca_works" != xyes; then
    AC_MSG_ERROR( [a system implementation of alloca is required] )
 fi
 
+
+HAVE_LIBMAGIC=no
+if test "${with_libmagic}" != "no"; then
+  #libmagic support
+  AC_CHECK_HEADERS(magic.h, [  
AC_CHECK_LIB(magic,magic_open,HAVE_LIBMAGIC=yes) ])
+fi
+
+if test "${HAVE_LIBMAGIC}" = "yes"; then
+  LIBMAGIC=-lmagic
+  AC_SUBST(LIBMAGIC)
+  AC_DEFINE(HAVE_LIBMAGIC, 1, [Define to 1 if using libmagic.])  
+fi
+
 # fmod, logb, and frexp are found in -lm on most systems.
 # On HPUX 9.01, -lm does not contain logb, so check for sqrt.
 AC_CHECK_LIB(m, sqrt)
@@ -2954,6 +2969,7 @@ echo "  Does Emacs use -lpng?                             
      ${HAVE_PNG}"
 echo "  Does Emacs use -lrsvg-2?                                ${HAVE_RSVG}"
 echo "  Does Emacs use -lgpm?                                   ${HAVE_GPM}"
 echo "  Does Emacs use -ldbus?                                  ${HAVE_DBUS}"
+echo "  Does Emacs use -lmagic?                                 
${HAVE_LIBMAGIC}"
 
 echo "  Does Emacs use -lfreetype?                              
${HAVE_FREETYPE}"
 echo "  Does Emacs use -lm17n-flt?                              
${HAVE_M17N_FLT}"
diff --git a/src/Makefile.in b/src/Makefile.in
index 425cf98..33d1a14 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -420,6 +420,7 @@ LIBX= $(LIBXMENU) LD_SWITCH_X_SITE
 #endif /* not HAVE_LIBRESOLV */
 
 LIBSOUND= @LIBSOUND@
+LIBMAGIC= @LIBMAGIC@
 CFLAGS_SOUND= @CFLAGS_SOUND@
 
 RSVG_LIBS= @RSVG_LIBS@
@@ -878,7 +879,7 @@ SOME_MACHINE_LISP = ../lisp/mouse.elc \
    duplicated symbols.  If the standard libraries were compiled
    with GCC, we might need gnulib again after them.  */
 
-LIBES = $(LOADLIBES) $(LIBS) $(LIBX) $(LIBSOUND) $(RSVG_LIBS) $(DBUS_LIBS) \
+LIBES = $(LOADLIBES) $(LIBS) $(LIBX) $(LIBSOUND) $(LIBMAGIC) $(RSVG_LIBS) 
$(DBUS_LIBS) \
    LIBGPM LIBRESOLV LIBS_SYSTEM LIBS_MACHINE LIBS_TERMCAP \
    LIBS_DEBUG $(GETLOADAVG_LIBS) \
    @FREETYPE_LIBS@ @FONTCONFIG_LIBS@ @LIBOTF_LIBS@ @M17N_FLT_LIBS@ \
diff --git a/src/fileio.c b/src/fileio.c
index 3702d4c..0061981 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -205,6 +205,10 @@ Lisp_Object Vdirectory_sep_char;
 int write_region_inhibit_fsync;
 #endif
 
+#ifdef HAVE_LIBMAGIC
+#include <magic.h>
+#endif
+
 /* Non-zero means call move-file-to-trash in Fdelete_file or
    Fdelete_directory.  */
 int delete_by_moving_to_trash;
@@ -2997,6 +3001,81 @@ DEFUN ("unix-sync", Funix_sync, Sunix_sync, 0, 0, "",
 
 #endif /* HAVE_SYNC */
 
+#ifdef HAVE_LIBMAGIC
+DEFUN ("libmagic-file-internal", Flibmagic_file_internal, 
Slibmagic_file_internal, 1,1,0,
+        doc: /* Return a list describing the argument FILENAME.
+
+
+  The return value is a list of the form 
+
+       (MIME-TYPE MIME-ENCODING-NAME DESCRIPTION)
+
+       MIME-TYPE and MIME-ENCODING-NAME are the MIME type and encoding
+  suitable for the file's contents, as determined by libmagic.
+  DESCRIPTION is the human readable descripton of the file type
+  offered by libmagic.
+
+  The function throws a file-error if libmagic cannot determine one of
+  the elements of the above list.
+
+  The default libmagic database is used, and the quality of
+  information given depends on your version of that database.  Often
+  the MIME type is less exact than the description.  */)
+  (filename)
+     Lisp_Object filename;
+{
+  magic_t cookie=NULL;
+  char* f = NULL;
+  const char* returnvaluestr;
+  Lisp_Object file_description;
+  Lisp_Object file_mime;
+  Lisp_Object file_encoding;  
+  Lisp_Object returnvalue;
+  Lisp_Object absname;
+  int errsave;
+  struct gcpro gcpro1, gcpro2;
+  
+  CHECK_STRING (filename);
+
+  GCPRO2 (filename, absname);
+  absname = Fexpand_file_name (filename, current_buffer->directory); //might gc
+  f = SDATA(ENCODE_FILE (absname));//might gc
+  UNGCPRO;
+  
+  cookie = magic_open (MAGIC_ERROR);
+  if (cookie == NULL) goto libmagic_error;  
+  magic_load (cookie, NULL); /* load default database */
+
+  magic_setflags (cookie, MAGIC_MIME_TYPE | MAGIC_ERROR);  
+  returnvaluestr = magic_file (cookie, f);
+  if (returnvaluestr == NULL) goto libmagic_error;
+  file_mime = intern (returnvaluestr);
+
+  magic_setflags (cookie, MAGIC_MIME_ENCODING | MAGIC_ERROR);
+  returnvaluestr=magic_file (cookie, f);
+  if (returnvaluestr == NULL) goto libmagic_error;
+  file_encoding = build_string(returnvaluestr);  
+
+  magic_setflags (cookie, MAGIC_NONE | MAGIC_ERROR);
+  returnvaluestr=magic_file (cookie, f);
+  if (returnvaluestr == NULL) goto libmagic_error;
+
+  file_description = build_string (returnvaluestr); 
+  returnvalue = Fcons (file_mime, Fcons (file_encoding, Fcons 
(file_description, Qnil))); //might gc
+  
+  magic_close (cookie);
+
+  return returnvalue;
+ libmagic_error:
+  UNGCPRO;
+  errsave=errno;
+  if (cookie != NULL) magic_close (cookie);
+  errno=errsave;
+  report_file_error("Libmagic error",Qnil);
+  return Qnil;
+}
+#endif
+
 DEFUN ("file-newer-than-file-p", Ffile_newer_than_file_p, 
Sfile_newer_than_file_p, 2, 2, 0,
        doc: /* Return t if file FILE1 is newer than file FILE2.
 If FILE1 does not exist, the answer is nil;
@@ -5781,6 +5860,9 @@ When non-nil, the function `move-file-to-trash' will be 
used by
 #ifdef HAVE_SYNC
   defsubr (&Sunix_sync);
 #endif
+#ifdef HAVE_LIBMAGIC
+    defsubr (&Slibmagic_file_internal);
+#endif
 }
 
 /* arch-tag: 64ba3fd7-f844-4fb2-ba4b-427eb928786c