I guess Gimp is bending the spec of xbm a bit here. To cite the
Xlib manual:

The X 
version 11 bitmap file format is: 

#define name_width width
#define 
name_height height
#define name_x_hot x
#define name_y_hot y
static 
unsigned char name_bits[] = { 0xNN,... }

Options are: 
 - treat this as a Gimp bug 
(thus CC'ing Sven)
 - be more forgiving and treat everything which looks like
   a C 
file as an xbm.
 - both

Other tools like ImageMagick seem to choke on initial 
comments
in xbms as well (display does display such an xbm, but only if
its 
filename ends in .xbm, thus I guess it falls back to 
guess-by-extension).
 

I have definitely seen programs other than Gimp that put C comments in
XBM files (though I can't offhand remember which programs they were...)

I'd sure have to come down on the "be lenient in what you accept" side
of things, here...

Yes, that makes sense as long as it doesn't interfere with  
other similarly looking 
image formats - we're doing content
sniffing here in order to determine if its an xbm. 
But fortunately,
xbm is the only C-source-based image format supported by
gdk-
pixbuf, so looking for /* in addition to #define seems
save.

You see, its easy to get this wrong: of course xpm is also
a C-based image format and 
starts with  /* XPM */. Thus
accepting /* for xbm is risky unless we're careful 
about
the order in which the image formats are tested. And I still
consider it a bug 
that the Gimp puts a comment there. It
doesn't do that for xpm, btw.

XBM is
m/^\s*(/\*.*?\*/\s*\n)?\#\s*define\s/s
or something very close.

To me this looks like a badly designed file format because of the lack
of a magic header. I don't see how you want to determine the file
format in this case. You'd try to load every C file using the XBM
loader and would risk to crash badly. Even if we would decide to
change The GIMP's XBM plug-in, there would still be lots of such files
out there.

I don't think you'll find a person alive who doesn't think the XBM
file format is a complete abomination.  But it exists.

Well, all I'm saying is that you shouldn't try to determine if a file
is XBM or not because there is no clean and reliable way of doing it.

What???  Then you're saying "you shouldn't load XBM files."  Forgive
me for thinking that's not the most helpful approach.

There is, of course, one completely reliable way of doing it: try to
parse it as an XBM and see if that gave you an image that had width,
height, and bits of a size to match.

I can't believe all the fuss you're making over "hey, ignore the /**/
comments when parsing."

I'm not saying you shouldn't load it. I'm only saying that you
shouldn't try to determine if the XBM pixbuf loader is appropriate by
looking at the first n bytes of the file. This just won't work for
file formats like XBM that don't identify themselves properly. It
might be reasonable to try the XBM loader if the file extension is ".xbm".

What failure mode, exactly, are you trying to avoid?

If EVERYTHING goes wrong, and someone feeds C source code into the
pixmap loader, what disaster will be averted by not even *trying* to
parse it as XBM and seeing if it works?

I was assuming that there's a variant for gdk_pixbuf_new_from_file()
that takes a type parameter to specify the loader to use. Hopefully I
missed something since I couldn't find such a variant. If it's really
missing, it needs to be added to be able to load files for which the
type can't be safely determined automatically.

Of course the XBM loader needs to be changed to parse C comments, but
I don't think you should try to change the file test to make it
recognize any possible XBM file since this is likely to fail.

gdk_pixbuf_loader_new_with_type is what you're looking for. But I don't think it 
makes much sense to disable automatic xbm detection completely. Determining file 
type by magic number can fail as well as detecting it by file extension, and guessing 
that something fed to an image loader which starts with #define or /* is either an xpm or 
an xbm file is not too bad. I've put a lot of effort into ensuring that the pixbuf loaders 
will properly report errors on any input and never ever crash the app. So the only 
visible change from not trying to autodected xbm should be that the error message 
changes from "failed to parse xbm" to "image format not supported".

I still think 
that the Gimp should try to create image formats according to their specs, no matter 
how bad those specs are. 

Removing the comments from the GIMP XBM plug-in would be a major
regression since we use it to save information about the cursor
hotspot with the XBM file. This is needed to edit our own cursor
files. What we could do is to move the comments to the end of the file
or place it after the defines.

I'd appreciate a pointer to the spec since I couldn't find it in the
documents Debian installs with the xspecs package.

Why would you put the hotspot coordinates in comments when the xbm format has explicit 
support for it ? This seems seriously wrong, but you may confuse this with a different 
gimp plugin, since my version of Gimp just puts a /* Created with The GIMP */ comment 
there, and the hotspot coordinates are saved as defines, as it should be. 

I 
already cited what "spec"-like information is contained in the Xlib manual.

Here 
is an online version: 
http://tronche.com/gui/x/xlib/utilities/manipulating-
bitmaps.html

Sorry, I remembered this wrong. Of course we use the defines to save
the cursor hotspot. Perhaps we should open a bug-report for The GIMP
and ask the maintainer of the XBM plug-in (Gordon Matzigkeit
<gord@gnu.org>) to give his opinion on the subject of comments in XBM
files.

Done (bug 82763). Couldn't cc gord@gnu.org though, as he doesn't seem
to have an 
account here.

It looks like the GIMP is not the only culprit.  I tried to look for
comments in XBM files on several systems and I found that about 10% of
the files had comments in them.  Here is a quick way to find them:
  locate .xbm | tr '\n' '\000' | xargs -n 1 --null fgrep -H '/*'

I found several files containing this comment (top of the file):
/* This X bitmap is designed for use with the X Desktop Manager.
 * it was designed by Alix Courtney, April 1991.
 */
If the date in the comment can be trusted, then these XBM files have
been around for quite a while.

In /usr/X11R6/include/X11/bitmaps/, there are several images of the
Linux penguin and of the BSD daemon.  All of them contain a comment
at the end of the file (not at the top).  Here is the BSD comment:
/*
 * BSD daemon
 * The original BSD daemon is Copyright (c) 1988 Marshall Kirk
McKusick.
 * All Rights Reserved.  Reproduced with permission.
 */

I found a bunch of icons designed by Connectiva in 2000, containing a
very large comment at the top of the file (the comment is larger than
the bitmap data).

Of course I also found a number of files containing the comment:
/* Created with The GIMP */
but most of these files were in the GIMP source tree.

So even if it would be better for the GIMP to write XBM files without
comments (or with a comment at the end of the file, which seems to
work with most readers), it might be a good idea for gdk-pixbuf to be
able to read XBM files containing comments.

To make things even more interesting, I have also found a machine that
has one file (/usr/lib/blt2.4/demos/bitmaps/xbob.xbm) starting with
one empty line before the first #define.