Commits

Anonymous committed 17405f2

Fixed gcc warnings about longjmp clobbering xyz

  • Participants
  • Parent commits 525ca88

Comments (0)

Files changed (2)

 
 #define TRY(expr) \
     do { \
-    if (setjmp(pimpl_->jmpbuf_) == 0) { expr; } \
-    pimpl_->ex_store_.rethrow_stored_exception(); \
+    if (setjmp(jmpbuf_) == 0) { expr; } \
+    ex_store_.rethrow_stored_exception(); \
     } while (false)
 
 namespace pngxx
                 std::longjmp(self(context).jmpbuf_, 42);
         }
 
+        // A bunch of the following functions exist purely to ensure that setjmp/longjmp
+        // don't 'clobber' any locals.
+
+        png_structp create_read_struct()
+        {
+            // We supply a custom allocator that records allocations. This means that it's fine to throw/longjmp
+            // from any callback we supply to libpng without worrying about leaks being caused by not returning
+            // to the calling function.
+            TRY
+            (
+                png_structp rs = png_create_read_struct_2(PNG_LIBPNG_VER_STRING, 
+                                                          this, &impl::error, &impl::warning, 
+                                                          &pool_, &allocate_in_pool, &free_from_pool);
+
+                if (rs) return rs;
+            );
+            throw std::bad_alloc();
+        }
+
+        png_infop create_info_struct(png_structp rs)
+        {
+            png_infop info = 0;
+            if (setjmp(jmpbuf_) == 0)
+                info = png_create_info_struct(rs);
+
+            if (ex_store_.has_exception() || !info)
+                png_destroy_read_struct(&rs, static_cast<png_infopp>(NULL), static_cast<png_infopp>(NULL));
+
+            ex_store_.rethrow_stored_exception();
+            if (!info) throw std::bad_alloc();
+
+            return info;
+        }
+
+        void set_read_function()
+        {
+            TRY( png_set_read_fn(context_, 0, &impl::read_bytes) );
+        }
+
+        void read_info()
+        {
+            TRY( png_read_info(context_, info_) );
+        }
+
+        void get_raster_info(png_uint_32 &width, png_uint_32 &height, int &bit_depth, int &colour_type, int &interlace_type, bool &alpha)
+        {
+            TRY( png_get_IHDR(context_, info_, &width, &height, &bit_depth, &colour_type, &interlace_type, 0, 0) );
+            TRY( alpha = (png_get_valid(context_, info_, PNG_INFO_tRNS) != 0) );
+        }
+
+        void get_resolution(png_uint_32 &pixels_per_m_x, png_uint_32 &pixels_per_m_y)
+        {
+            TRY( pixels_per_m_x = png_get_x_pixels_per_meter(context_, info_) );
+            TRY( pixels_per_m_y = png_get_y_pixels_per_meter(context_, info_) );
+        }
+
+        void prepare_for_reading(int colour_type, int bit_depth, imagexx::pixel_format format)
+        {
+            TRY( png_set_strip_16(context_) ); // 16 bit pixels read as 8 bit pixels
+            TRY( png_set_packing(context_) ); // unpack 1-bit, 2-bit and 4-bit pixels in to their own bytes
+
+            if (colour_type & PNG_COLOR_TYPE_PALETTE)
+                TRY( png_set_palette_to_rgb(context_) ); // a paletted image should come through as an RGB raster
+
+            if ((colour_type & PNG_COLOR_TYPE_GRAY) && bit_depth < 8)
+                TRY( png_set_expand_gray_1_2_4_to_8(context_) ); // expand 1, 2, 4 bpp greyscale images to 8 bpp
+
+            if (format == imagexx::rgba || format == imagexx::grey_alpha)
+                TRY( png_set_tRNS_to_alpha(context_) ); // if image has transparency, read pixels as RGBA or GA
+
+            TRY( passes_ = png_set_interlace_handling(context_) );
+        }
+
+        void read_row(unsigned char *out)
+        {
+            TRY( png_read_row(context_, out, NULL) );
+        }
+
+        void read_image(unsigned char **out)
+        {
+            TRY( png_read_image(context_, out) );
+        }
+
         std::auto_ptr<imagexx::source> source_;
 
         png_structp context_;
 
     void loader::reset()
     {
-        png_structp rs = 0;
-
-        // We supply a custom allocator that records allocations. This means that it's fine to throw/longjmp
-        // from any callback we supply to libpng without worrying about leaks being caused by not returning 
-        // to the calling function.
-        TRY(rs = png_create_read_struct_2(PNG_LIBPNG_VER_STRING, 
-                                          pimpl_.get(), &impl::error, &impl::warning, 
-                                          &pimpl_->pool_, &allocate_in_pool, &free_from_pool));
-        if (!rs) throw std::bad_alloc();
-
-        png_infop info = 0;
-        if (setjmp(pimpl_->jmpbuf_) == 0)
-            info = png_create_info_struct(rs);
-        if (pimpl_->ex_store_.has_exception() || !info)
-            png_destroy_read_struct(&rs, static_cast<png_infopp>(NULL), static_cast<png_infopp>(NULL));
-
-        pimpl_->ex_store_.rethrow_stored_exception();
-        if (!info) throw std::bad_alloc();
+        png_structp rs = pimpl_->create_read_struct();
+        png_infop info = pimpl_->create_info_struct(rs);
 
         clear();
 
         assert(src.get());
         pimpl_->source_ = src;
         assert(pimpl_->context_);
-        TRY(png_set_read_fn(pimpl_->context_, 0, &impl::read_bytes));
+        pimpl_->set_read_function();
     }
 
     void loader::read_details()
         assert(pimpl_->context_);
         assert(pimpl_->source_.get());
 
-        png_structp rsp = pimpl_->context_;
-
-        TRY(png_read_info(rsp, pimpl_->info_));
+        pimpl_->read_info();
 
         png_uint_32 width = 0;
         png_uint_32 height = 0;
+        int bit_depth = 0;
         int colour_type = 0;
-        int bit_depth = 0;
         int interlace_type = 0;
         bool alpha_in_tRNS = false;
 
-        TRY(png_get_IHDR(rsp, pimpl_->info_, &width, &height, &bit_depth, &colour_type, &interlace_type, 0, 0));
-        TRY(alpha_in_tRNS = (png_get_valid(rsp, pimpl_->info_, PNG_INFO_tRNS) != 0));
+        pimpl_->get_raster_info(width, height, bit_depth, colour_type, interlace_type, alpha_in_tRNS);
 
         imagexx::pixel_format format = imagexx::rgb;
 
             else format = imagexx::grey;
         }
 
-        const png_uint_32 pixels_per_mm_x = png_get_x_pixels_per_meter(rsp, pimpl_->info_);
-        const png_uint_32 pixels_per_mm_y = png_get_y_pixels_per_meter(rsp, pimpl_->info_);
-        const double width_mm = pixels_per_mm_x ? 1000.0 * width / pixels_per_mm_x : 0.0;
-        const double height_mm = pixels_per_mm_y ? 1000.0 * height / pixels_per_mm_y : 0.0;
+        png_uint_32 pixels_per_m_x = 0;
+        png_uint_32 pixels_per_m_y = 0;
+        pimpl_->get_resolution(pixels_per_m_x, pixels_per_m_y);
+        const double width_mm = pixels_per_m_x ? 1000.0 * width / pixels_per_m_x : 0.0;
+        const double height_mm = pixels_per_m_y ? 1000.0 * height / pixels_per_m_y : 0.0;
 
         pimpl_->details_ = imagexx::raster_details(format, width, height, width_mm, height_mm);
 
         // Prepare to start reading the raster
-        TRY(png_set_strip_16(rsp)); // 16 bit pixels read as 8 bit pixels
-        TRY(png_set_packing(rsp)); // unpack 1-bit, 2-bit and 4-bit pixels in to their own bytes
-
-        if (colour_type & PNG_COLOR_TYPE_PALETTE)
-            TRY(png_set_palette_to_rgb(rsp)); // a paletted image should come through as an RGB raster
-
-        if ((colour_type & PNG_COLOR_TYPE_GRAY) && bit_depth < 8)
-            TRY(png_set_expand_gray_1_2_4_to_8(rsp)); // expand 1, 2, 4 bpp greyscale images to 8 bpp
-
-        if (format == imagexx::rgba || format == imagexx::grey_alpha)
-            TRY(png_set_tRNS_to_alpha(rsp)); // if image has transparency, read pixels as RGBA or GA
-
-        TRY(pimpl_->passes_ = png_set_interlace_handling(rsp));
+        pimpl_->prepare_for_reading(colour_type, bit_depth, format);
     }
 
     void loader::read_row(std::vector<unsigned char> &row)
     {
-        TRY(png_read_row(pimpl_->context_, &row.front(), NULL));
+        if (!row.empty()) // perhaps it's not possible to have zero-size PNGs, but just in case.
+            pimpl_->read_row(&row.front());
     }
 
     void loader::read_entire_raster(std::vector<unsigned char> &raster)
         for (std::size_t r = 0, h = d.height(); r != h; ++r)
             rows[r] = &raster[stride * r];
 
-        TRY(png_read_image(pimpl_->context_, &rows.front()));
+        if (!rows.empty()) // perhaps it's not possible to have zero-size PNGs, but just in case.
+            pimpl_->read_image(&rows.front());
     }
 
     bool loader::interlaced() const
 
 #define TRY(expr) \
     do { \
-    if (setjmp(pimpl_->jmpbuf_) == 0) expr; \
-    pimpl_->ex_store_.rethrow_stored_exception(); \
+    if (setjmp(jmpbuf_) == 0) { expr; } \
+    ex_store_.rethrow_stored_exception(); \
     } while (false)
 
 namespace pngxx
             {
             }
 
+            // A bunch of the following functions exist purely to ensure that setjmp/longjmp
+            // don't 'clobber' any locals.
+
+            void create_write_struct()
+            {
+                // We supply a custom allocator that records allocations. This means that it's fine to throw/longjmp
+                // from any callback we supply to libpng without worrying about leaks being caused by not returning
+                // to the calling function.
+                TRY
+                (
+                    context_ = png_create_write_struct_2(PNG_LIBPNG_VER_STRING, 
+                                                         this, &impl::error, &impl::warning,
+                                                         &pool_, &allocate_in_pool, &free_from_pool)
+                );
+                if (!context_) throw std::bad_alloc();
+            }
+
+            void create_info_struct()
+            {
+                TRY( info_ = png_create_info_struct(context_) );
+                if (!info_) throw std::bad_alloc();
+            }
+
+            void set_write_function()
+            {
+                TRY( png_set_write_fn(context_, 0, &impl::write_bytes, &impl::flush) );
+            }
+
+            void set_compression_level(int level)
+            {
+                TRY( png_set_compression_level(context_, level) );
+            }
+
+            void set_IHDR(const imagexx::raster_details &details, int colour_type)
+            {
+                TRY
+                (
+                    png_set_IHDR(context_, info_, 
+                                 details.width(), details.height(),
+                                 8, colour_type,
+                                 PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT)
+                );
+            }
+
+            void set_sBIT(const png_color_8 &colour_bits)
+            {
+                TRY( png_set_sBIT(context_, info_, &colour_bits) );
+            }
+
+            void set_pHYs(png_uint_32 pixels_per_m_x, png_uint_32 pixels_per_m_y)
+            {
+                TRY( png_set_pHYs(context_, info_, pixels_per_m_x, pixels_per_m_y, PNG_RESOLUTION_METER) );
+            }
+
+            void write_info()
+            {
+                TRY( png_write_info(context_, info_) );
+            }
+
+            void write_end()
+            {
+                TRY( png_write_end(context_, info_) );
+            }
+
+            void write_row(const unsigned char *bytes)
+            {
+                TRY
+                (
+                    unsigned char *row = const_cast<unsigned char *>(bytes); // Grr!
+                    png_write_row(context_, row)
+                );
+            }
+
             std::auto_ptr<imagexx::sink> sink_;
             png_structp context_;
             png_infop info_;
             pimpl_->sink_ = snk;
             assert(pimpl_->sink_.get());
 
-            png_structp ws = 0;
-            png_infop info = 0;
-            
-            // We supply a custom allocator that records allocations. This means that it's fine to throw/longjmp
-            // from any callback we supply to libpng without worrying about leaks being caused by not returning
-            // to the calling function.
-            TRY(ws = png_create_write_struct_2(PNG_LIBPNG_VER_STRING, 
-                                               pimpl_.get(), &impl::error, &impl::warning,
-                                               &pimpl_->pool_, &allocate_in_pool, &free_from_pool));
-            if (!ws) throw std::bad_alloc();
-
-            pimpl_->context_ = ws;
-
-            TRY(info = png_create_info_struct(ws));
-            if (!info) throw std::bad_alloc();
-
-            pimpl_->info_ = info;
-            
-            TRY(png_set_write_fn(ws, 0, &impl::write_bytes, &impl::flush));
+            pimpl_->create_write_struct();
+            pimpl_->create_info_struct();
+            pimpl_->set_write_function();
 
             if (compression < 0.0) compression = 1.0;
             else if (compression > 1.0) compression = 0.0;
 
-            if (compression == 0) png_set_compression_level(ws, Z_NO_COMPRESSION);
-            else if (compression < 0.5) png_set_compression_level(ws, Z_BEST_SPEED);
-
-            TRY(png_set_compression_level(ws, Z_BEST_COMPRESSION));
+            if (compression == 0) pimpl_->set_compression_level(Z_NO_COMPRESSION);
+            else if (compression < 0.5) pimpl_->set_compression_level(Z_BEST_SPEED);
+            else pimpl_->set_compression_level(Z_BEST_COMPRESSION);
 
             typedef imagexx::raster_details rdetails;
             int colour_type = PNG_COLOR_MASK_COLOR;
                 default: break;
             }
 
-            TRY(png_set_IHDR(ws, info,
-                             details.width(), details.height(),
-                             8, colour_type,
-                             PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT));
+            pimpl_->set_IHDR(details, colour_type);
 
             png_color_8 colour_bits;
             if (colour_type & PNG_COLOR_MASK_COLOR)
             if (colour_type & PNG_COLOR_MASK_ALPHA)
                 colour_bits.alpha = 8;
 
-            TRY(png_set_sBIT(ws, info, &colour_bits));
+            pimpl_->set_sBIT(colour_bits);
 
             if (details.width_mm() && details.height_mm())
             {
                 const double pixels_per_m_x = 1000.0 * details.width() / details.width_mm();
                 const double pixels_per_m_y = 1000.0 * details.height() / details.height_mm();
-                TRY(png_set_pHYs(ws, info, 
-                                 pngxx::round(pixels_per_m_x), pngxx::round(pixels_per_m_y), PNG_RESOLUTION_METER));
+                pimpl_->set_pHYs(pngxx::round(pixels_per_m_x), pngxx::round(pixels_per_m_y));
             }
 
-            TRY(png_write_info(ws, info));
+            pimpl_->write_info();
         }
 
         writer::~writer()
 
         void writer::finish()
         {
-            TRY(png_write_end(pimpl_->context_, pimpl_->info_));
+            pimpl_->write_end();
         }
 
         void writer::write_row(const unsigned char *bytes)
         {
-            unsigned char *row = const_cast<unsigned char *>(bytes); // Grr!
-            TRY(png_write_row(pimpl_->context_, row));
+            pimpl_->write_row(bytes);
         }
 
     } // close namespace detail