21.5
Vin, this presumably affects 21.4 too. If you don't recall any
related reports (probably only comes up if PNG lib that gets found at
runtime differs from build-time link lib), I'll take my time....
Stephen J. Turnbull writes:
Does `configure --with-cppflags="-newflag $CPPFLAGS"
...' work in your
application? I think that having the `--with-whateverflags' options
override the corresponding environment variables is easier to
explain. It's certainly easier to code!
[[ This is in my queue but I am working on other stuff. ]]
Ron Isaacson writes:
> #2 is definitely not our problem. :-) I think you need to
ensure that
> png_err_stct.setjmp_buffer is set to a valid address BEFORE passing
> png_error_func as a parameter to anything. Otherwise, any runtime
> error from png_create_read_struct will get you into the same boat.
OK, yes, this is really ugly. (But then, when isn't a setjmp/longjmp
ugly?) I think the attached patch should fix the problem, but I don't
yet have a test case and I've never patched code involving setjmp/
longjmp before so a second opinion would be welcome (and a third...).
(I think the relevant test case for the original bug can be easily
constructed by substituting "You're going down, sucker!" for
PNG_LIBPNG_VER_STRING, but I haven't done that yet.)
The first hunk is a little suspicious; for some reason
png_destroy_read_struct() wants access to the pointer variable
containing png_ptr, not just the pointer itself. The only reason I
can think why it would want that is to NULL it, and I've done that, so
passing &tmp to png_destroy_read_struct() should be OK, I think.
The other changes are to first NULL out any unwind pointers to
structures that are being destroyed in case the libpng code longjmps,
then call libpng's finalizer; to move the setjmp call much earlier;
and to stuff the unwind structure with the the necessary data as soon
as we get it.
I guess I have to assume that none of the libpng functions will
implicitly call a finalizer....
I'll run with it a while before committing, but would really
appreciate comments.
2007-02-17 Stephen J. Turnbull <stephen(a)xemacs.org>
* glyphs-eimage.c (png_instantiate_unwind): Avoid recursion.
(png_instantiate): Initialize setjmp_buffer early, and avoid
recursive entry to error handler.
diff --git a/src/glyphs-eimage.c b/src/glyphs-eimage.c
index 49364ca..5184e2c 100644
--- a/src/glyphs-eimage.c
+++ b/src/glyphs-eimage.c
@@ -849,7 +849,13 @@ png_instantiate_unwind (Lisp_Object unwind_obj)
free_opaque_ptr (unwind_obj);
if (data->png_ptr)
- png_destroy_read_struct (&(data->png_ptr), &(data->info_ptr),
(png_infopp)NULL);
+ {
+ /* ensure we can't get here again */
+ png_structp tmp = data->png_ptr;
+ data->png_ptr = NULL;
+ png_destroy_read_struct (&tmp, &(data->info_ptr), (png_infopp)NULL);
+ }
+
if (data->instream)
retry_fclose (data->instream);
@@ -874,24 +880,36 @@ png_instantiate (Lisp_Object image_instance, Lisp_Object
instantiator,
png_structp png_ptr;
png_infop info_ptr;
+ xzero (unwind);
+ record_unwind_protect (png_instantiate_unwind, make_opaque_ptr (&unwind));
+
+ if (setjmp (png_err_stct.setjmp_buffer))
+ {
+ /* Something blew up:
+ just display the error (cleanup happens in the unwind) */
+ signal_image_error_2 ("Error decoding PNG",
+ build_string(png_err_stct.err_str),
+ instantiator);
+ }
+
/* Initialize all PNG structures */
- png_ptr = png_create_read_struct (PNG_LIBPNG_VER_STRING, (void*)&png_err_stct,
+ png_ptr = png_create_read_struct (PNG_LIBPNG_VER_STRING,
+ (void *) &png_err_stct,
png_error_func, png_warning_func);
if (!png_ptr)
signal_image_error ("Error obtaining memory for png_read", instantiator);
+ unwind.png_ptr = png_ptr;
+
info_ptr = png_create_info_struct (png_ptr);
if (!info_ptr)
{
+ unwind.png_ptr = NULL; /* avoid re-calling png_destroy_read_struct
+ when unwinding */
png_destroy_read_struct (&png_ptr, (png_infopp)NULL, (png_infopp)NULL);
signal_image_error ("Error obtaining memory for png_read",
instantiator);
}
-
- xzero (unwind);
- unwind.png_ptr = png_ptr;
unwind.info_ptr = info_ptr;
- record_unwind_protect (png_instantiate_unwind, make_opaque_ptr (&unwind));
-
/* This code is a mixture of stuff from Ben's GIF/JPEG stuff from
this file, example.c from the libpng 0.81 distribution, and the
pngtopnm sources. -WMP-
@@ -900,16 +918,6 @@ png_instantiate (Lisp_Object image_instance, Lisp_Object
instantiator,
and is no longer usable for previous versions. jh
*/
- /* Set the jmp_buf return context for png_error ... if this returns !0, then
- we ran into a problem somewhere, and need to clean up after ourselves. */
- if (setjmp (png_err_stct.setjmp_buffer))
- {
- /* Something blew up: just display the error (cleanup happens in the unwind) */
- signal_image_error_2 ("Error decoding PNG",
- build_string(png_err_stct.err_str),
- instantiator);
- }
-
/* Initialize the IO layer and read in header information */
{
Lisp_Object data = find_keyword_in_vector (instantiator, Q_data);
_______________________________________________
XEmacs-Beta mailing list
XEmacs-Beta(a)xemacs.org
http://calypso.tux.org/cgi-bin/mailman/listinfo/xemacs-beta