Created attachment 141527 [details] [review]
Revert back to letting g_content_type_guess do its job

This is visible in the game crack-attack, which has a GTK front-end with .tga
format screen dumps.  For some time the screen dumps have been replaced with a
broken image icon.  The fault comes down to
gdk-pixbuf/gdk-pixbuf-io.c:_gdk_pixbuf_get_module() - which goes to
considerable effort to not let g_content_type_guess() do the job it is trying
to do.

The code used to work.  The change is apparently in response to "bug"
http://bugzilla.gnome.org/show_bug.cgi?id=566862

The logic in that bug report is deeply questionable.  And the final applied
patch (as presciently questioned in Bug#566862 comment 5) is even more
questionable.  g_content_type_guess() can do a better job if the file name is
provided.

g_content_type_guess() could reasonably be expected to do more.  For example,
"bug" 566862 could be fixed in g_content_type_guess() by using the extension
.jpg, double checking against the magic for .jpg, and if it doesn't match it
could fall back onto the matching magic for .png.

But matching only by magic is fraught with danger.  It may usually do the
right thing for most of the file formats that GTK supports.  But in the case
of crack-attack it mistakes .tga files for Win32 cursor files, and tries in
vain to parse the file with the "ico" loader rather than the "tga" loader.

Whether "bug" 566862 is a real problem or not is debatable.  But of course
it's obvious that gdk-pixbuf-io.c:_gdk_pixbuf_get_module() shouldn't be trying
to do (getting in the way of) g_content_type_guess()'s job.  The current code
thinks that it can do a better job than g_content_type_guess() by calling it
multiple times with different parameters.

If _gdk_pixbuf_get_module()'s algorithm is correct, it should be in
g_content_type_guess().  But, as demonstrated by the .tga/.cur format problem,
it's not correct.

This patch fixes the .tga file display in the crack-attack front-end.

The problem is that:

1. The .tga files start with the magic "\0  \0 002  \0  \0"

2. This matches the TGA mime rule in
   /usr/pkg/share/mime/packages/freedesktop.org.xml (or, rather, in the binary
   files generated from that source), but it ALSO matches the
   image/x-win-bitmap Windows cursor magic, with a higher priority.

3. The higher priority match is probably correct, since more bytes are
   checked.  Arguably devel/glib2/gio/gcontenttype.c:g_content_type_guess()
   should flag that the match is uncertain - but it doesn't.

4. gdk-pixbuf/gdk-pixbuf-io.c calls g_content_type_guess() without a filename
   first, and only passes the filename if the magic check is "uncertain".

5. devel/glib2/gio/gcontenttype.c:g_content_type_guess() only ever finds the
   topmost match via magic, so the "uncertain" check almost never flags as
   uncertain - certainly not if there multiple magic matches.

6. Note that in the case of .tga files, there is only one registered file type
   - TGA images, so the filename check would easily determine the right file
   type.

7. The end result is that the wrong image loader is loaded, and fails to parse
   the image (/usr/pkg/lib/gtk-2.0/2.10.0/loaders/libpixbufloader-ico.so
   instead of /usr/pkg/lib/gtk-2.0/2.10.0/loaders/libpixbufloader-tga.so).
   The application displays a broken image icon.