comments on libmagic

[Christos Zoulas]

On Oct 14, 11:02am, brettf at deepfile.com ("Brett Funderburg") wrote:
-- Subject: comments on libmagic

| I've been playing around with libmagic over the past few days and I'd like
| to offer the following comments for feedback.
| 
| I've noticed some inconsistencies in how the api works. For example,
| 
| Case 1
| 
| (pseudo-code)
| 
| magic_open()
| result = magic_file("/path/to/some/file")
| if !result
|    print magic_error()  <- "No magic files loaded"

Yes, this is because this is an internal magic error.
 
| Case 2
| 
| magic_open()
| magic_load()
| result = magic_file("/file/not/found")
| print magic_error()  <- ""
| print result         <- "Can't stat file..."

Yes, because this is what file(1) is supposed to return. Actually by posix
it is supposed to return "not found".

| Case 1 works as I would expect it to. I think it's preferable in Case 2 to
| have it work like the following:
| 
| magic_open()
| magic_load()
| result = magic_file("/file/not/found")
| if !result
|    print magic_error()  <- "Can't stat file"
| 
| Indeed, I think this is how the man page describes how it should work.
| However, this points out an additional limitation. Relying on simple 0,
| -1, NULL return values from library functions does not provide enough
| information to the caller about what has gone wrong. Textual error
| descriptions provided by magic_error are fine but aren't suitable for
| error handling. As a consumer of the libmagic api, this is what I would
| like to do:
| 
| my_app_get_file_type(char* file_path) {
| 
| ... set up libmagic stuff ...
| 
| result = magic_file(file_path);
| if(result != NULL)
|     return result;    <- "ELF executable, etc."
| 
|   error = magic_errno()
|   switch(error) {
|   case STAT_ERROR:
|     ... handle this case ...
|     print magic_error()  <- "Doesn't exist..."
| 
|   case OPEN_ERROR:
|    ... handle this other case ...
|     print magic_error()  <- "Can't open..."
|   }
| }
| 
| Comments?
| 
| brett

I see your point. I think that the best solution is to give you access to
the errno information, and change the default behavior via a flag:

	ms = magic_open(MAGIG_ERROR);

	if ((result = magic_file(ms, fn)) == NULL) {
		int error = magic_errno(ms);
		if (error == 0)
			// File internal error
			fprintf("%s: %s\n", fn, file_error(ms));
		else
			// Os error
			fprintf("%s: %s\n", fn, strerror(error));
	}

In fact, I have implemented all this already...

christos