XBM loader is too strict on variable name

[orzel]

Pillow fails to open almost all my xbm files.
The reason is the xbm_head in Pillow-12.3.0/src/PIL/XbmImagePlugin.py

It hard-codes the fact that the variable name must ends with "_bits", which is maybe a common
practice, but definitely not a requirement.

So this will load:

#define hopper_width 128
#define hopper_height 128
static char hopper_bits[] = {...

But this one won't:

#define hopper_width 128
#define hopper_height 128
static char hopper_xbm[] = {...

_xbm is another common suffix, but please don't restrict to only _xbm and _bits!

[orzel]

It's weird, because your regexp doesn't require static and char, which are a lot more typical (if not mandatory) for xbm, but wants _bits ?? Typical values is static unsigned char whatever[] ={
Also, a lot of xbm files have (c-style) comments at beginning, and your plugin will fail on them too:

/*
This is an example file
*/
#define hopper_width 128
#define hopper_height 128
static char hopper_bits[] = {

[teachershuang]

I can take the array-name part of this issue. On current main (9d307290), I built Pillow 13.0.0.dev0 locally: the existing six XBM tests and 59 selftests pass, but an 8x1 XBM with static unsigned char probe_xbm[] = {0x55}; fails through Image.open().

I'll remove the _bits suffix requirement while retaining the width/height parsing, header read limit, hotspot handling, and decoder. The tests will load renamed versions of an existing XBM fixture and compare the decoded pixels, including identifiers without either common suffix and the existing _bits case. I'll include a release note.

The leading-comment report affects format detection as well as header parsing; I'll keep that separate from this small naming fix. My local build has the core and zlib, but not the optional JPEG/WebP/etc. libraries, so broader platform/codec validation will need CI. I use AI assistance and will run the focused tests, coverage, lint and independent diff review before submitting a Draft PR.

[hugovk]

Is there an XBM spec or reference implementation we can compare with?

[teachershuang]

@hugovk I checked two original implementations, and they differ:

• Xlib's XReadBitmapFileData extracts the part after the last underscore and requires bits[]. That accepts foo_bits[] or bare bits[], but not foo_xbm[] or an arbitrary name.
• ImageMagick's ReadXBMImage recognizes the static short/char/unsigned-char declaration, then locates the opening brace; it does not enforce a suffix on the captured array name. Its writer still emits _bits.

I have not found a formal specification establishing arbitrary names as a requirement, and I inspected these sources rather than executing their readers locally. So #10088 is a permissive-reader compatibility extension for the reported files, not a claim of strict Xlib conformance. It preserves Pillow's existing _bits path and writer.

Would you welcome that compatibility extension, or prefer retaining Xlib's naming restriction? I'll keep the PR Draft pending that direction.

[radarhere]

https://www.fileformat.info/format/xbm/egff.htm describes

static unsigned char FOO_bits[] = {

That, to me, is intended to communicate that the FOO prefix is arbitrary, but _bits is not.

_xbm is another common suffix, but please don't restrict to only _xbm and _bits!

It sounds like you are familiar with XBM images. I presume you were inspired to create this issue because you found an image with _xbm. What software was used to create that?

[radarhere]

I have found https://github.com/syntelos/xbm/blob/master/README.txt

The three primary symbols, width, height and bits have the conventional string format: <prefix> + '_' + ("width"|"height"|"bits").

So _bits is in the specification. Unless you can offer any evidence that there are encoders out there ignoring this, I don't think this change is needed.

[radarhere]

From teachershuang - #10088 (comment)

ImageMagick accepting arbitrary names is reader evidence, not proof of a real encoder emitting them. I have not identified a verified producer.