New filemagic patch mostly fixing Eli:s concerns.


Eli Zaretskii <eliz@gnu.org> writes:
>> +DEFUN ("libmagic-file-internal", Flibmagic_file_internal, Slibmagic_file_internal, 1,1,0,
>> +       doc: /* Return (MIME_TYPE MIME_ENCODING DESCRIPTION) for FILENAME_OR_BUFFER.
>> +Return nil on error. */)


Renamed entry point to libmagic-file-internal since its meant to be
of internal usage for a lisp wrapper, yet to be written. Should that be
a new file BTW?

> This doc string "needs work"(TM).  Please use the doc string of
> visited-file-name as an example.

I worked on this

>
>> +  (filename_or_buffer)
>> +     Lisp_Object filename_or_buffer;
>
> Using a `_' in an argument is un-Lisp'y (IMO).

Ok.

> You need to encode file names before you pass them to C APIs.  Use
> ENCODE_FILE to do that; see file-attributes for an example of how this
> is done.

Ok.

>> +  if (rvs == NULL) goto libmagic_error;
>> +  Lisp_Object file_mime = intern (rvs);
>
> You cannot declare variables in the middle of a block: Emacs does not
> require a C99 compiler yet and need to support C90 or even older
> compilers, which will reject this code.

I'm habing trouble remembering not to use c99. Is there some convenient
compiler flag to force lower versions? Fixed the errors I saw.


> Is file_encoding supposed to be a valid encoding, one of those for
> which Emacs has a coding-system?  If so, perhaps you should make sure
> you indeed return a valid coding-system or its alias, or otherwise
> tell in the doc string that it's not guaranteed to be valid (so that
> the caller should validate it before using).

I described a bit more in the doc string. Ok?