GNOME Bugzilla – Bug 82763
xbm plugin emits malformed xbms
Last modified: 2009-08-15 18:40:50 UTC
See bug 82706 for full details. In short, the xbm file format doesn't allow initial C comments.
As already noted by Sven in bug #82706, it might be acceptable to move the comment to the end of the file. I don't know if it would still cause some problems with XBM loaders that are less forgiving than the GIMP.
Moving the comments to the end of the file isn't that trivial. Of course it would be easy to write it out at the end of the file, but the load function expects the comment before the header. I don't think we can easily change this without the risk of breaking the code. So it's probably a 1.3.x issue.
From my reading of the XLib docs (which are, as always, open to correction), there doesn't seem to be any provision at all in xbm for comments. http://www.msu.edu/~huntharo/xwin/docs/xwindows/XLIB.pdf is the XLib spec, and xbm is specified in the docs for the XReadBitmapFile. Can't we suppress the writing (and reading) of comments completely? Or does the comment need to be there? Dave.
The first thing that we could do as a quick patch for 1.2.4 is to suppress the writing of the comments. But we should definitely not suppress the reading part, because this code regression would break the plug-in for all images that were saved with previous versions of the GIMP. If we want to follow the guideline "be strict in what you produce and be forgiving in what you accept", then we should modify the xbm plug-in in 1.2.x and 1.3.x so that it never writes any comments but it can still read the old "GIMP XBM" files with comments. Note that the GIMP is not the only application that adds comments in XBM files. See my comment in bug #82706. Suggestion: - Change the plug-in in 1.2.4 so that it does not write any comments (this is a one-line patch). - Make sure that the plug-in in 1.2.4 does not crash if there is a comment at the end of the file. This is already working, so no other changes are necessary. - Do the same in CVS HEAD. - Later, change the plug-in in CVS HEAD so that it writes the comments at the end of the file. Allow it to read the comments from the top and from the bottom of the file. I don't think that we would loose much if the XBM plug-in in 1.2.4 is silently ignoring the comments at the end of the file, because they are not used frequently and they are not very important anyway.
If we drop writing of comments from 1.2, we also have to remove the user interface that allows to edit the comment. Anyway, it's probably the best we can do. Who writes the patch?
Go on, then - I'll do it. So you want the text box in the dialog chopped out, and the writing of the comment suppressed? Attachment to come once I run a test. Dave.
Created attachment 8704 [details] [review] Patch to xbm to suppress writing of comments. No changes on inut side.
I have written a patch that is IMHO a bit better: it leaves all the original code intact (including the function prototypes) so that it is easier to add the comments back in CVS HEAD (but written at the end of the file). I ifdef'ed the parts of the code that display the comment entry in the dialog box and I also disabled the line that writes the comment to the file. By the way, I noticed that the current version of the plug-in does not work with multi-line comments and that it limits the size of the comment to 72 characters, which would break many of the existing XBM files that have longer comments (as reported in bug #82706). The patch is attached below.
Created attachment 8705 [details] [review] patch to disable writing of comments and input field
Bug #82706 has been fixed in the meantime. The latest version of gdk-pixbuf reads our XBM files. Should we nevertheless apply Raphaels patch?
It depends on how many programs have problems with non-standard comments included at the top of the file. The GTK programs based on gdk-pixbuf were probably not the only ones. If we want to follow the XBM standard, then we should not include any comment at the top of the file. But if there are not too many programs affected by these comments, then we can ignore the patch that I submitted earlier.
I say apply the patch, make the GIMP do the right thing while still accepting that others do the wrong thing. Dave.
Go ahead and apply it then. Whoever wants a comment in his/her XBM files can easily add it by hand.
2002-11-07 Dave Neary <bolsh@gimp.org> * plug-ins/common/xbm.c: Applied patch from Raphael Quinet to suppress printing of comments in XBM files. Closes bug #82763.
Obviously what was committed to CVS is not exactly what was discussed here. The version in CVS had a lot more code commented out than what Raphaels patch suggested. The change from Bolsh effectively disabled the GUI for specifying hotspots and mask files. I have reverted this part of the change.
Just for the record, I committed this change to gimp-1-2: 2003-05-26 Sven Neumann <sven@gimp.org> * plug-ins/common/xbm.c (save_dialog): Reverted some of the changes that Dave Neary committed on 2002-11-07 since they disabled some useful features without a good reason (see bug #82763).
The fix is part of the stable release 1.2.5. Closing this bug.