[gdk-pixbuf/wip/rishi/png] io-png: Clarify and fix the error handling

[Debarshi Ray <debarshir src gnome org>]

commit aa7d203ec63ed1e14ac558415b26fe3e42ab8e44
Author: Debarshi Ray <debarshir gnome org>
Date:   Sun Sep 9 20:26:22 2018 +0200

    io-png: Clarify and fix the error handling
    
    Neither png_create_read_struct* nor png_create_info_struct can trigger
    a longjmp out of libpng. That's only documented as being possible with
    png_set_progressive_read_fn, even though, currently, it does not.
    
    The documentation [1] is being a bit simplistic when it says:
      "... you will need to update the jmpbuf field every time you enter a
       new routine that will call a png_*() function"
    
    On closer look, the documentation for png_create_read_struct* [2] and
    png_create_info_struct [3] clarify that they return NULL on error and
    not longjmp. This is borne out by the libpng code, comments and
    examples.
    
    The current location of the setjmp is confusing to the reader because
    one can be misled into thinking that there's no need to check the
    value returned by png_create_info_struct, or that a failure will lead
    to the error handler and a longjmp.
    
    In light of this, the assumption that the error callback would set the
    GError when either png_create_read_struct* or png_create_info_struct
    fails is wrong. These functions can only fail if there was a request
    to allocate an absurdly large amount of memory that exceeds one of
    limits encoded in libpng; or if the memory allocator returns NULL; or
    if there was a ABI mismatch caused by compiling and running against
    incompatible libpng versions. All those are programming or
    integration errors, and normally one wouldn't bother handling them.
    However, since the libpng documentation repeatedly mentions that the
    return value from these functions should be checked, it's wise to do
    so.
    
    Fallout from e8d0d8ed3d33ee6cedb75a394c36af3312b310ff
    
    [1] http://www.libpng.org/pub/png/libpng-1.2.5-manual.html#section-3
    [2] 
http://refspecs.linuxbase.org/LSB_4.1.0/LSB-Desktop-generic/LSB-Desktop-generic/libpng12.png.create.read.struct.1.html
    [3] 
http://refspecs.linuxbase.org/LSB_4.1.0/LSB-Desktop-generic/LSB-Desktop-generic/libpng12.png.create.info.struct.1.html

 gdk-pixbuf/io-png.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)
---
diff --git a/gdk-pixbuf/io-png.c b/gdk-pixbuf/io-png.c
index 06cf36595..e39b1ecb7 100644
--- a/gdk-pixbuf/io-png.c
+++ b/gdk-pixbuf/io-png.c
@@ -483,16 +483,19 @@ gdk_pixbuf__png_image_begin_load (GdkPixbufModuleSizeFunc size_func,
 #endif
         if (lc->png_read_ptr == NULL) {
                 g_free(lc);
-                /* error callback should have set the error */
+
+                /* A failure here isn't supposed to call the error
+                 * callback, but it doesn't hurt to be careful.
+                 */
+                if (error && *error == NULL) {
+                        g_set_error_literal (error,
+                                             GDK_PIXBUF_ERROR,
+                                             GDK_PIXBUF_ERROR_INSUFFICIENT_MEMORY,
+                                             _("Couldn’t allocate memory for loading PNG"));
+                }
+
                 return NULL;
         }
-        
-       if (setjmp (png_jmpbuf(lc->png_read_ptr))) {
-                png_destroy_read_struct(&lc->png_read_ptr, &lc->png_info_ptr, NULL);
-                g_free(lc);
-                /* error callback should have set the error */
-                return NULL;
-       }
 
         /* Create the auxiliary context struct */
 
@@ -501,6 +504,23 @@ gdk_pixbuf__png_image_begin_load (GdkPixbufModuleSizeFunc size_func,
         if (lc->png_info_ptr == NULL) {
                 png_destroy_read_struct(&lc->png_read_ptr, NULL, NULL);
                 g_free(lc);
+
+                /* A failure here isn't supposed to call the error
+                 * callback, but it doesn't hurt to be careful.
+                 */
+                if (error && *error == NULL) {
+                        g_set_error_literal (error,
+                                             GDK_PIXBUF_ERROR,
+                                             GDK_PIXBUF_ERROR_INSUFFICIENT_MEMORY,
+                                             _("Couldn’t allocate memory for loading PNG"));
+                }
+
+                return NULL;
+        }
+
+        if (setjmp (png_jmpbuf(lc->png_read_ptr))) {
+                png_destroy_read_struct(&lc->png_read_ptr, &lc->png_info_ptr, NULL);
+                g_free(lc);
                 /* error callback should have set the error */
                 return NULL;
         }