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: [[ This is in my queue but I am working on other stuff. ]] 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&gt; * 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-Patches mailing list XEmacs-Patches(a)xemacs.org http://calypso.tux.org/cgi-bin/mailman/listinfo/xemacs-patches