GNOME Bugzilla – Bug 405539
gdk_pixbuf_save() for PNG saver can return FALSE without filling the GError
Last modified: 2010-07-10 04:06:47 UTC
Please describe the problem: If gdk_pixbuf_save() is called for the PNG saver with unrecognized parameters (e.g. "compression" for versions prior to 2.8.0), it will return FALSE without filling the GError object. I also noticed that cleanup was not happening if png_create_write_struct() failed, causing potential leakage if memory had been allocated for the png_text. A patch fixing these issues is attached. Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
Created attachment 82121 [details] [review] Fill GError if returning FALSE and plug leak
Created attachment 82129 [details] [review] Fill GError if returning FALSE and plug leak I noticed the same issue in the JPEG loader, so I updated the patch to include the fix for that too.
I have committed the leak fix. Wrt. the error handling, I believe that when we added the error handling in the pixbuf saving code, we decided that invalid parameter values are recoverable runtime errors (reportable through GError) while invalid keys are programming errors (reportable via g_error, g_warning, etc).
The problem with invalid keys being considered programming errors is that when new keys are added (e.g. "compression" being added to the png saver in 2.8.0), the code will compile on previous versions just fine, but will not work. I know we can't change this retroactively, but it seems like it might be a good idea going forward to use g_warning() to indicate unrecognized keys, but to ignore them. Otherwise there needs to be version checking when using new parameters.
Thats a valid point, although I don't expect new keys for existing loaders to appear at any rate that would be a cause for concern... Can you cook a patch that ignores unknown keys with a warning ?
Created attachment 82688 [details] [review] Patch to warn and continue when unrecognized parameters are encountered during a pixbuf save. (Also a tiny JPEG saver leak fix)
2007-04-28 Matthias Clasen <mclasen@redhat.com> * io-png.c: * io-jpeg.c: Accept unknown parameters with a warning when saving. (#405539, Daniel Atallah)