Gdk-pixbuf loader issues

[Federico Mena Quintero <federico helixcode com>]

One of the missing things before gdk-pixbuf can go 1.0 is to make the
loaders robust.  Ideally they would have full error reporting, but we
cannot do this without breaking the API quite a bit.

Still, some of the loader code calls g_error() as a form of throwing
its hands up in the air when it doesn't know what to do.  This is
Bad(tm), especially for applications that load images from unknown
sources (read:  mail attachments in Evolution); we don't want any
random broken image to crash the program.

(In general g_error() is useless except for when your program wants to
bail out because its internal consistency got screwed up).

These are the places where g_error() is being used incorrectly:

io-gif.c:535 -----------------------------------------

	} else if (code == context->lzw_end_code) {
		int count;
		unsigned char buf[260];

		/*g_error (" DID WE EVER EVER GET HERE\n");*/
		g_error ("Unhandled Case.  If you have an image that causes this, let me <jrb@redhat.com> know.\n");

io-jpeg.c:466 ----------------------------------------

	context->pixbuf = gdk_pixbuf_new(GDK_COLORSPACE_RGB, 
					 FALSE,
					 8, 
					 cinfo->image_width,
					 cinfo->image_height);

	if (context->pixbuf == NULL) {
		/* Failed to allocate memory */
		g_error ("Couldn't allocate gdkpixbuf");
	}


io-pnm.c:649 ----------------------------------------

	context.pixels = g_malloc (context.height * 
				   context.width  * 3);
	if (!context.pixels) {
		/* Failed to allocate memory */
		g_error ("Couldn't allocate pixel buf");
	}

io-pnm.c:810 ----------------------------------------

	context->pixbuf = gdk_pixbuf_new(GDK_COLORSPACE_RGB, 
					 FALSE,
					 8, 
					 context->width,
					 context->height);

	if (context->pixbuf == NULL) {
		/* Failed to allocate memory */
		g_error ("Couldn't allocate gdkpixbuf");
	}


The first one should be simply replaced by a g_message, and it should
be (forcibly?) tested so that the program's internal state does not
get corrupted and it recovers gracefully.

The other ones should really let the calling program know that memory
was exausted.  In this case, I don't care if we have to change the
API; crashing is just not an option.

The progressive loaders should store an internal flag that says
whether everything is OK up to the current loading point.  If
something fails, they should emit a signal or otherwise notify the
calling program that something is amiss.

Right now we have

	gboolean
	gdk_pixbuf_loader_write (GdkPixbufLoader *loader, 
				 const guchar *buf, 
				 size_t count);

I think we should replace the return value by an enumeration, or make
it void and add an argument similar to GException.  Maybe the latter
is better.

If a loader exhausts its memory before being able to allocate the
pixbuf, it should return NULL if gdk_pixbuf_loader_get_pixbuf() is
called on it later.  If it was able to create the pixbuf, but runs out
of memory in the middle of loading the image, it should simply set the
exception to the proper value and abort the loading process.

So, who volunteers to make these changes? :-)

  Federico