Follow-on cleanup for bug #271401. Isolates setjmp(), removes early return, and allow...

diff --git a/src/sp-image.cpp b/src/sp-image.cpp
index 8ef7969cd10d877a45242ee8dcee530b64b8b8a8..93e7b4d65844370cdb47ddd407eebca0b8542212 100644 (file)
--- a/src/sp-image.cpp
+++ b/src/sp-image.cpp
@@ -262,6 +262,161 @@ void user_flush_data( png_structp /*png_ptr*/ )
     //g_message( "user_flush_data" );
 }
 
+
+static bool readPngAndHeaders( PushPull &youme, gint & dpiX, gint & dpiY )
+{
+    bool good = true;
+
+    gboolean isPng = !png_sig_cmp( youme.scratch + youme.offset, 0, youme.available() );
+    //g_message( "  png? %s", (isPng ? "Yes":"No") );
+    if ( isPng ) {
+        png_structp pngPtr = png_create_read_struct( PNG_LIBPNG_VER_STRING,
+                                                     0, //(png_voidp)user_error_ptr,
+                                                     0, //user_error_fn,
+                                                     0 //user_warning_fn
+            );
+        png_infop infoPtr = pngPtr ? png_create_info_struct( pngPtr ) : 0;
+
+        if ( pngPtr && infoPtr ) {
+            if ( setjmp(png_jmpbuf(pngPtr)) ) {
+                // libpng calls longjmp to return here if an error occurs.
+                good = false;
+            }
+
+            if (good) {
+                png_set_read_fn( pngPtr, &youme, user_read_data );
+                //g_message( "In" );
+
+                //png_read_info( pngPtr, infoPtr );
+                png_read_png( pngPtr, infoPtr, PNG_TRANSFORM_IDENTITY, 0 );
+
+                //g_message("out");
+
+                /*
+                  if ( png_get_valid( pngPtr, infoPtr, PNG_INFO_pHYs ) )
+                  {
+                  g_message("pHYs chunk now valid" );
+                  }
+                  if ( png_get_valid( pngPtr, infoPtr, PNG_INFO_sCAL ) )
+                  {
+                  g_message("sCAL chunk now valid" );
+                  }
+                */
+
+                png_uint_32 res_x = 0;
+                png_uint_32 res_y = 0;
+                int unit_type = 0;
+                if ( png_get_pHYs( pngPtr, infoPtr, &res_x, &res_y, &unit_type) ) {
+//                                     g_message( "pHYs yes (%d, %d) %d (%s)", (int)res_x, (int)res_y, unit_type,
+//                                                (unit_type == 1? "per meter" : "unknown")
+//                                         );
+
+//                                     g_message( "    dpi: (%d, %d)",
+//                                                (int)(0.5 + ((double)res_x)/39.37),
+//                                                (int)(0.5 + ((double)res_y)/39.37) );
+                    if ( unit_type == PNG_RESOLUTION_METER )
+                    {
+                        // TODO come up with a more accurate DPI setting
+                        dpiX = (int)(0.5 + ((double)res_x)/39.37);
+                        dpiY = (int)(0.5 + ((double)res_y)/39.37);
+                    }
+                } else {
+//                                     g_message( "pHYs no" );
+                }
+
+/*
+  double width = 0;
+  double height = 0;
+  int unit = 0;
+  if ( png_get_sCAL(pngPtr, infoPtr, &unit, &width, &height) )
+  {
+  gchar* vals[] = {
+  "unknown", // PNG_SCALE_UNKNOWN
+  "meter", // PNG_SCALE_METER
+  "radian", // PNG_SCALE_RADIAN
+  "last", //
+  NULL
+  };
+
+  g_message( "sCAL: (%f, %f) %d (%s)",
+  width, height, unit,
+  ((unit >= 0 && unit < 3) ? vals[unit]:"???")
+  );
+  }
+*/
+
+#if defined(PNG_sRGB_SUPPORTED)
+                {
+                    int intent = 0;
+                    if ( png_get_sRGB(pngPtr, infoPtr, &intent) ) {
+//                                         g_message("Found an sRGB png chunk");
+                    }
+                }
+#endif // defined(PNG_sRGB_SUPPORTED)
+
+#if defined(PNG_cHRM_SUPPORTED)
+                {
+                    double white_x = 0;
+                    double white_y = 0;
+                    double red_x = 0;
+                    double red_y = 0;
+                    double green_x = 0;
+                    double green_y = 0;
+                    double blue_x = 0;
+                    double blue_y = 0;
+
+                    if ( png_get_cHRM(pngPtr, infoPtr,
+                                      &white_x, &white_y,
+                                      &red_x, &red_y,
+                                      &green_x, &green_y,
+                                      &blue_x, &blue_y) ) {
+//                                         g_message("Found a cHRM png chunk");
+                    }
+                }
+#endif // defined(PNG_cHRM_SUPPORTED)
+
+#if defined(PNG_gAMA_SUPPORTED)
+                {
+                    double file_gamma = 0;
+                    if ( png_get_gAMA(pngPtr, infoPtr, &file_gamma) ) {
+//                                         g_message("Found a gAMA png chunk");
+                    }
+                }
+#endif // defined(PNG_gAMA_SUPPORTED)
+
+#if defined(PNG_iCCP_SUPPORTED)
+                {
+                    char* name = 0;
+                    int compression_type = 0;
+                    char* profile = 0;
+                    png_uint_32 proflen = 0;
+                    if ( png_get_iCCP(pngPtr, infoPtr, &name, &compression_type, &profile, &proflen) ) {
+//                                         g_message("Found an iCCP chunk named [%s] with %d bytes and comp %d", name, proflen, compression_type);
+                    }
+                }
+#endif // defined(PNG_iCCP_SUPPORTED)
+
+            }
+        } else {
+            g_message("Error when creating PNG read struct");
+        }
+
+        // now clean it up.
+        if (pngPtr && infoPtr) {
+            png_destroy_read_struct( &pngPtr, &infoPtr, 0 );
+            pngPtr = 0;
+            infoPtr = 0;
+        } else if (pngPtr) {
+            png_destroy_read_struct( &pngPtr, 0, 0 );
+            pngPtr = 0;
+        }
+    } else {
+        good = false; // Was not a png file
+    }
+
+    return good;
+}
+
 static GdkPixbuf* pixbuf_new_from_file( const char *filename, time_t &modTime, gchar*& pixPath, GError **/*error*/ )
 {
     GdkPixbuf* buf = NULL;
@@ -297,10 +452,6 @@ static GdkPixbuf* pixbuf_new_from_file( const char *filename, time_t &modTime, g
             // short buffer
             guchar scratch[1024];
             gboolean latter = FALSE;
-            gboolean isPng = FALSE;
-            png_structp pngPtr = NULL;
-            png_infop infoPtr = NULL;
-            //png_infop endPtr = NULL;
 
             youme.fp = fp;
             youme.scratch = scratch;
@@ -309,167 +460,20 @@ static GdkPixbuf* pixbuf_new_from_file( const char *filename, time_t &modTime, g
             youme.offset = 0;
             youme.loader = loader;
 
-            while ( !feof(fp) )
+            bool dropOut = false;
+            while ( !feof(fp) && !dropOut )
             {
-                if ( youme.readMore() )
-                {
-                    if ( youme.first )
-                    {
+                if ( youme.readMore() ) {
+                    if ( youme.first ) {
                         //g_message( "First data chunk" );
                         youme.first = FALSE;
-                        isPng = !png_sig_cmp( scratch + youme.offset, 0, youme.available() );
-                        //g_message( "  png? %s", (isPng ? "Yes":"No") );
-                        if ( isPng )
+                        if (!readPngAndHeaders(youme, dpiX, dpiY))
                         {
-                            pngPtr = png_create_read_struct( PNG_LIBPNG_VER_STRING,
-                                                             NULL,//(png_voidp)user_error_ptr,
-                                                             NULL,//user_error_fn,
-                                                             NULL//user_warning_fn
-                                );
-                            if ( pngPtr )
-                            {
-                                if ( setjmp(png_jmpbuf(pngPtr)) )
-                                {
-                                    // libpng calls longjmp to return here if an error occurs.
-                                    png_destroy_read_struct( &pngPtr, &infoPtr, NULL );
-                                    fclose(fp);
-                                    gdk_pixbuf_loader_close(loader, NULL);
-                                    g_object_unref(loader);
-                                    return NULL;
-                                }
-
-                                infoPtr = png_create_info_struct( pngPtr );
-                                //endPtr = png_create_info_struct( pngPtr );
-
-                                png_set_read_fn( pngPtr, &youme, user_read_data );
-                                //g_message( "In" );
-
-                                //png_read_info( pngPtr, infoPtr );
-                                png_read_png( pngPtr, infoPtr, PNG_TRANSFORM_IDENTITY, NULL );
-
-                                //g_message("out");
-
-                                //png_read_end(pngPtr, endPtr);
-
-                                /*
-                                if ( png_get_valid( pngPtr, infoPtr, PNG_INFO_pHYs ) )
-                                {
-                                    g_message("pHYs chunk now valid" );
-                                }
-                                if ( png_get_valid( pngPtr, infoPtr, PNG_INFO_sCAL ) )
-                                {
-                                    g_message("sCAL chunk now valid" );
-                                }
-                                */
-
-                                png_uint_32 res_x = 0;
-                                png_uint_32 res_y = 0;
-                                int unit_type = 0;
-                                if ( png_get_pHYs( pngPtr, infoPtr, &res_x, &res_y, &unit_type) )
-                                {
-//                                     g_message( "pHYs yes (%d, %d) %d (%s)", (int)res_x, (int)res_y, unit_type,
-//                                                (unit_type == 1? "per meter" : "unknown")
-//                                         );
-
-//                                     g_message( "    dpi: (%d, %d)",
-//                                                (int)(0.5 + ((double)res_x)/39.37),
-//                                                (int)(0.5 + ((double)res_y)/39.37) );
-                                    if ( unit_type == PNG_RESOLUTION_METER )
-                                    {
-                                        // TODO come up with a more accurate DPI setting
-                                        dpiX = (int)(0.5 + ((double)res_x)/39.37);
-                                        dpiY = (int)(0.5 + ((double)res_y)/39.37);
-                                    }
-                                }
-                                else
-                                {
-//                                     g_message( "pHYs no" );
-                                }
-
-/*
-                                double width = 0;
-                                double height = 0;
-                                int unit = 0;
-                                if ( png_get_sCAL(pngPtr, infoPtr, &unit, &width, &height) )
-                                {
-                                    gchar* vals[] = {
-                                        "unknown", // PNG_SCALE_UNKNOWN
-                                        "meter", // PNG_SCALE_METER
-                                        "radian", // PNG_SCALE_RADIAN
-                                        "last", //
-                                        NULL
-                                    };
-
-                                    g_message( "sCAL: (%f, %f) %d (%s)",
-                                               width, height, unit,
-                                               ((unit >= 0 && unit < 3) ? vals[unit]:"???")
-                                        );
-                                }
-*/
-
-#if defined(PNG_sRGB_SUPPORTED)
-                                {
-                                    int intent = 0;
-                                    if ( png_get_sRGB(pngPtr, infoPtr, &intent) ) {
-//                                         g_message("Found an sRGB png chunk");
-                                    }
-                                }
-#endif // defined(PNG_sRGB_SUPPORTED)
-
-#if defined(PNG_cHRM_SUPPORTED)
-                                {
-                                    double white_x = 0;
-                                    double white_y = 0;
-                                    double red_x = 0;
-                                    double red_y = 0;
-                                    double green_x = 0;
-                                    double green_y = 0;
-                                    double blue_x = 0;
-                                    double blue_y = 0;
-
-                                    if ( png_get_cHRM(pngPtr, infoPtr,
-                                                      &white_x, &white_y,
-                                                      &red_x, &red_y,
-                                                      &green_x, &green_y,
-                                                      &blue_x, &blue_y) ) {
-//                                         g_message("Found a cHRM png chunk");
-                                    }
-                                }
-#endif // defined(PNG_cHRM_SUPPORTED)
-
-#if defined(PNG_gAMA_SUPPORTED)
-                                {
-                                    double file_gamma = 0;
-                                    if ( png_get_gAMA(pngPtr, infoPtr, &file_gamma) ) {
-//                                         g_message("Found a gAMA png chunk");
-                                    }
-                                }
-#endif // defined(PNG_gAMA_SUPPORTED)
-
-#if defined(PNG_iCCP_SUPPORTED)
-                                {
-                                    char* name = 0;
-                                    int compression_type = 0;
-                                    char* profile = 0;
-                                    png_uint_32 proflen = 0;
-                                    if ( png_get_iCCP(pngPtr, infoPtr, &name, &compression_type, &profile, &proflen) ) {
-//                                         g_message("Found an iCCP chunk named [%s] with %d bytes and comp %d", name, proflen, compression_type);
-                                    }
-                                }
-#endif // defined(PNG_iCCP_SUPPORTED)
-
-
-                                // now clean it up.
-                                png_destroy_read_struct( &pngPtr, &infoPtr, NULL );//&endPtr );
-                            }
-                            else
-                            {
-//                                 g_message("Error when creating PNG read struct");
-                            }
+                            // We failed to read the PNG headers and info.
+                            // The GDK pixbuf loader may still get something.
+                            dropOut = true;
                         }
-                    }
-                    else if ( !latter )
-                    {
+                    } else if ( !latter ) {
                         latter = TRUE;
                         //g_message("  READing latter");
                     }
@@ -480,52 +484,40 @@ static GdkPixbuf* pixbuf_new_from_file( const char *filename, time_t &modTime, g
             }
 
             gboolean ok = gdk_pixbuf_loader_close(loader, &err);
-            if ( ok )
-            {
+            if ( ok ) {
                 buf = gdk_pixbuf_loader_get_pixbuf( loader );
-                if ( buf )
-                {
+                if ( buf ) {
                     g_object_ref(buf);
 
-                    if ( dpiX )
-                    {
+                    if ( dpiX ) {
                         gchar *tmp = g_strdup_printf( "%d", dpiX );
-                        if ( tmp )
-                        {
-//                             g_message("Need to set DpiX: %s", tmp);
+                        if ( tmp ) {
+                            //g_message("Need to set DpiX: %s", tmp);
                             //gdk_pixbuf_set_option( buf, "Inkscape::DpiX", tmp );
                             g_free( tmp );
                         }
                     }
-                    if ( dpiY )
-                    {
+                    if ( dpiY ) {
                         gchar *tmp = g_strdup_printf( "%d", dpiY );
-                        if ( tmp )
-                        {
-//                             g_message("Need to set DpiY: %s", tmp);
+                        if ( tmp ) {
+                            //g_message("Need to set DpiY: %s", tmp);
                             //gdk_pixbuf_set_option( buf, "Inkscape::DpiY", tmp );
                             g_free( tmp );
                         }
                     }
                 }
-            }
-            else
-            {
+            } else {
                 // do something
                 g_message("error loading pixbuf at close");
             }
 
             g_object_unref(loader);
-        }
-        else
-        {
+        } else {
             g_message("error when creating pixbuf loader");
         }
         fclose( fp );
-        fp = NULL;
-    }
-    else
-    {
+        fp = 0;
+    } else {
         g_warning ("Unable to open linked file: %s", filename);
     }