From 40bce90baad677cd631c27819b32cca9c5d3a1ab Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 12 Sep 2012 14:34:24 -0700 Subject: [PATCH 1/1] More fixes for 'volatile' and setjmp/longjmp. * eval.c (Fdefvar, Fcondition_case): Remove unnecessary 'volatile's. * image.c (struct png_load_context) [HAVE_PNG]: New type. (png_load_body) [HAVE_PNG]: (jpeg_load_body) [HAVE_JPEG]: New function, with most of the old parent function's body. (png_load) [HAVE_PNG]: (jpeg_load) [HAVE_JPEG]: Invoke the new function, to avoid longjmp munging our locals. (struct my_jpeg_error_mgr) [HAVE_JPEG]: New members cinfo, failure_code. (my_error_exit) [HAVE_JPEG]: Don't trust 'setjmp' to return 2 when longjmp is passed 2, as the C standard doesn't guarantee this. Instead, store the failure code into mgr->failure_code. --- src/ChangeLog | 16 +++++ src/eval.c | 11 ++- src/image.c | 196 +++++++++++++++++++++++++++++++------------------- 3 files changed, 144 insertions(+), 79 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 8b47c52c23..c0d3316bea 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,19 @@ +2012-09-12 Paul Eggert + + More fixes for 'volatile' and setjmp/longjmp. + * eval.c (Fdefvar, Fcondition_case): Remove unnecessary 'volatile's. + * image.c (struct png_load_context) [HAVE_PNG]: New type. + (png_load_body) [HAVE_PNG]: + (jpeg_load_body) [HAVE_JPEG]: + New function, with most of the old parent function's body. + (png_load) [HAVE_PNG]: + (jpeg_load) [HAVE_JPEG]: + Invoke the new function, to avoid longjmp munging our locals. + (struct my_jpeg_error_mgr) [HAVE_JPEG]: New members cinfo, failure_code. + (my_error_exit) [HAVE_JPEG]: Don't trust 'setjmp' to return 2 when + longjmp is passed 2, as the C standard doesn't guarantee this. + Instead, store the failure code into mgr->failure_code. + 2012-09-12 Stefan Monnier * keyboard.c (read_char, requeued_events_pending_p, Finput_pending_p) diff --git a/src/eval.c b/src/eval.c index 8a8a507a1b..3c0c65e936 100644 --- a/src/eval.c +++ b/src/eval.c @@ -707,7 +707,7 @@ usage: (defvar SYMBOL &optional INITVALUE DOCSTRING) */) else { /* Check if there is really a global binding rather than just a let binding that shadows the global unboundness of the var. */ - volatile struct specbinding *pdl = specpdl_ptr; + struct specbinding *pdl = specpdl_ptr; while (pdl > specpdl) { if (EQ ((--pdl)->symbol, sym) && !pdl->func @@ -1204,12 +1204,9 @@ See also the function `signal' for more info. usage: (condition-case VAR BODYFORM &rest HANDLERS) */) (Lisp_Object args) { - register Lisp_Object bodyform, handlers; - volatile Lisp_Object var; - - var = Fcar (args); - bodyform = Fcar (Fcdr (args)); - handlers = Fcdr (Fcdr (args)); + Lisp_Object var = Fcar (args); + Lisp_Object bodyform = Fcar (Fcdr (args)); + Lisp_Object handlers = Fcdr (Fcdr (args)); return internal_lisp_condition_case (var, bodyform, handlers); } diff --git a/src/image.c b/src/image.c index cf01602050..cd5df99ef5 100644 --- a/src/image.c +++ b/src/image.c @@ -5591,20 +5591,31 @@ png_read_from_file (png_structp png_ptr, png_bytep data, png_size_t length) /* Load PNG image IMG for use on frame F. Value is non-zero if successful. */ +struct png_load_context +{ + /* These are members so that _longjmp doesn't munge local variables. */ + png_struct *png_ptr; + png_info *info_ptr; + png_info *end_info; + FILE *fp; + png_byte *pixels; + png_byte **rows; +}; + static int -png_load (struct frame *f, struct image *img) +png_load_body (struct frame *f, struct image *img, struct png_load_context *c) { Lisp_Object file, specified_file; Lisp_Object specified_data; int x, y; ptrdiff_t i; XImagePtr ximg, mask_img = NULL; - png_struct *png_ptr = NULL; + png_struct *png_ptr; png_info *info_ptr = NULL, *end_info = NULL; - FILE *volatile fp = NULL; + FILE *fp = NULL; png_byte sig[8]; - png_byte * volatile pixels = NULL; - png_byte ** volatile rows = NULL; + png_byte *pixels = NULL; + png_byte **rows = NULL; png_uint_32 width, height; int bit_depth, color_type, interlace_type; png_byte channels; @@ -5671,24 +5682,26 @@ png_load (struct frame *f, struct image *img) png_ptr = fn_png_create_read_struct (PNG_LIBPNG_VER_STRING, NULL, my_png_error, my_png_warning); - if (!png_ptr) + if (png_ptr) { - if (fp) fclose (fp); - return 0; + info_ptr = fn_png_create_info_struct (png_ptr); + end_info = fn_png_create_info_struct (png_ptr); } - info_ptr = fn_png_create_info_struct (png_ptr); - if (!info_ptr) + c->png_ptr = png_ptr; + c->info_ptr = info_ptr; + c->end_info = end_info; + c->fp = fp; + c->pixels = pixels; + c->rows = rows; + + if (! (info_ptr && end_info)) { - fn_png_destroy_read_struct (&png_ptr, NULL, NULL); - if (fp) fclose (fp); - return 0; + fn_png_destroy_read_struct (&c->png_ptr, &c->info_ptr, &c->end_info); + png_ptr = 0; } - - end_info = fn_png_create_info_struct (png_ptr); - if (!end_info) + if (! png_ptr) { - fn_png_destroy_read_struct (&png_ptr, &info_ptr, NULL); if (fp) fclose (fp); return 0; } @@ -5698,14 +5711,18 @@ png_load (struct frame *f, struct image *img) if (_setjmp (PNG_JMPBUF (png_ptr))) { error: - if (png_ptr) - fn_png_destroy_read_struct (&png_ptr, &info_ptr, &end_info); - xfree (pixels); - xfree (rows); - if (fp) fclose (fp); + if (c->png_ptr) + fn_png_destroy_read_struct (&c->png_ptr, &c->info_ptr, &c->end_info); + xfree (c->pixels); + xfree (c->rows); + if (c->fp) + fclose (c->fp); return 0; } + /* Silence a bogus diagnostic; see GCC bug 54561. */ + IF_LINT (fp = c->fp); + /* Read image info. */ if (!NILP (specified_data)) fn_png_set_read_fn (png_ptr, (void *) &tbr, png_read_from_memory); @@ -5821,8 +5838,8 @@ png_load (struct frame *f, struct image *img) if (min (PTRDIFF_MAX, SIZE_MAX) / sizeof *rows < height || min (PTRDIFF_MAX, SIZE_MAX) / sizeof *pixels / height < row_bytes) memory_full (SIZE_MAX); - pixels = xmalloc (sizeof *pixels * row_bytes * height); - rows = xmalloc (height * sizeof *rows); + c->pixels = pixels = xmalloc (sizeof *pixels * row_bytes * height); + c->rows = rows = xmalloc (height * sizeof *rows); for (i = 0; i < height; ++i) rows[i] = pixels + i * row_bytes; @@ -5832,7 +5849,7 @@ png_load (struct frame *f, struct image *img) if (fp) { fclose (fp); - fp = NULL; + c->fp = NULL; } /* Create an image and pixmap serving as mask if the PNG image @@ -5907,7 +5924,7 @@ png_load (struct frame *f, struct image *img) #endif /* COLOR_TABLE_SUPPORT */ /* Clean up. */ - fn_png_destroy_read_struct (&png_ptr, &info_ptr, &end_info); + fn_png_destroy_read_struct (&c->png_ptr, &c->info_ptr, &c->end_info); xfree (rows); xfree (pixels); @@ -5936,6 +5953,13 @@ png_load (struct frame *f, struct image *img) return 1; } +static int +png_load (struct frame *f, struct image *img) +{ + struct png_load_context c; + return png_load_body (f, img, &c); +} + #else /* HAVE_PNG */ #ifdef HAVE_NS @@ -6106,6 +6130,16 @@ struct my_jpeg_error_mgr { struct jpeg_error_mgr pub; jmp_buf setjmp_buffer; + + /* The remaining members are so that _longjmp doesn't munge local + variables. */ + struct jpeg_decompress_struct cinfo; + enum + { + MY_JPEG_ERROR_EXIT, + MY_JPEG_INVALID_IMAGE_SIZE, + MY_JPEG_CANNOT_CREATE_X + } failure_code; }; @@ -6113,6 +6147,7 @@ static _Noreturn void my_error_exit (j_common_ptr cinfo) { struct my_jpeg_error_mgr *mgr = (struct my_jpeg_error_mgr *) cinfo->err; + mgr->failure_code = MY_JPEG_ERROR_EXIT; _longjmp (mgr->setjmp_buffer, 1); } @@ -6319,17 +6354,15 @@ jpeg_file_src (j_decompress_ptr cinfo, FILE *fp) from the JPEG lib. */ static int -jpeg_load (struct frame *f, struct image *img) +jpeg_load_body (struct frame *f, struct image *img, + struct my_jpeg_error_mgr *mgr) { - struct jpeg_decompress_struct cinfo; - struct my_jpeg_error_mgr mgr; Lisp_Object file, specified_file; Lisp_Object specified_data; - FILE * volatile fp = NULL; + FILE *fp = NULL; JSAMPARRAY buffer; int row_stride, x, y; XImagePtr ximg = NULL; - int rc; unsigned long *colors; int width, height; @@ -6361,24 +6394,33 @@ jpeg_load (struct frame *f, struct image *img) /* Customize libjpeg's error handling to call my_error_exit when an error is detected. This function will perform a longjmp. */ - cinfo.err = fn_jpeg_std_error (&mgr.pub); - mgr.pub.error_exit = my_error_exit; - - if ((rc = _setjmp (mgr.setjmp_buffer)) != 0) + mgr->cinfo.err = fn_jpeg_std_error (&mgr->pub); + mgr->pub.error_exit = my_error_exit; + if (_setjmp (mgr->setjmp_buffer)) { - if (rc == 1) + switch (mgr->failure_code) { - /* Called from my_error_exit. Display a JPEG error. */ - char buf[JMSG_LENGTH_MAX]; - cinfo.err->format_message ((j_common_ptr) &cinfo, buf); - image_error ("Error reading JPEG image `%s': %s", img->spec, - build_string (buf)); + case MY_JPEG_ERROR_EXIT: + { + char buf[JMSG_LENGTH_MAX]; + mgr->cinfo.err->format_message ((j_common_ptr) &mgr->cinfo, buf); + image_error ("Error reading JPEG image `%s': %s", img->spec, + build_string (buf)); + break; + } + + case MY_JPEG_INVALID_IMAGE_SIZE: + image_error ("Invalid image size (see `max-image-size')", Qnil, Qnil); + break; + + case MY_JPEG_CANNOT_CREATE_X: + break; } /* Close the input file and destroy the JPEG object. */ if (fp) - fclose ((FILE *) fp); - fn_jpeg_destroy_decompress (&cinfo); + fclose (fp); + fn_jpeg_destroy_decompress (&mgr->cinfo); /* If we already have an XImage, free that. */ x_destroy_x_image (ximg); @@ -6390,44 +6432,47 @@ jpeg_load (struct frame *f, struct image *img) /* Create the JPEG decompression object. Let it read from fp. Read the JPEG image header. */ - fn_jpeg_CreateDecompress (&cinfo, JPEG_LIB_VERSION, sizeof (cinfo)); + fn_jpeg_CreateDecompress (&mgr->cinfo, JPEG_LIB_VERSION, sizeof *&mgr->cinfo); if (NILP (specified_data)) - jpeg_file_src (&cinfo, (FILE *) fp); + jpeg_file_src (&mgr->cinfo, fp); else - jpeg_memory_src (&cinfo, SDATA (specified_data), + jpeg_memory_src (&mgr->cinfo, SDATA (specified_data), SBYTES (specified_data)); - fn_jpeg_read_header (&cinfo, 1); + fn_jpeg_read_header (&mgr->cinfo, 1); /* Customize decompression so that color quantization will be used. Start decompression. */ - cinfo.quantize_colors = 1; - fn_jpeg_start_decompress (&cinfo); - width = img->width = cinfo.output_width; - height = img->height = cinfo.output_height; + mgr->cinfo.quantize_colors = 1; + fn_jpeg_start_decompress (&mgr->cinfo); + width = img->width = mgr->cinfo.output_width; + height = img->height = mgr->cinfo.output_height; if (!check_image_size (f, width, height)) { - image_error ("Invalid image size (see `max-image-size')", Qnil, Qnil); - _longjmp (mgr.setjmp_buffer, 2); + mgr->failure_code = MY_JPEG_INVALID_IMAGE_SIZE; + _longjmp (mgr->setjmp_buffer, 1); } /* Create X image and pixmap. */ if (!x_create_x_image_and_pixmap (f, width, height, 0, &ximg, &img->pixmap)) - _longjmp (mgr.setjmp_buffer, 2); + { + mgr->failure_code = MY_JPEG_CANNOT_CREATE_X; + _longjmp (mgr->setjmp_buffer, 1); + } /* Allocate colors. When color quantization is used, - cinfo.actual_number_of_colors has been set with the number of - colors generated, and cinfo.colormap is a two-dimensional array - of color indices in the range 0..cinfo.actual_number_of_colors. + mgr->cinfo.actual_number_of_colors has been set with the number of + colors generated, and mgr->cinfo.colormap is a two-dimensional array + of color indices in the range 0..mgr->cinfo.actual_number_of_colors. No more than 255 colors will be generated. */ { int i, ir, ig, ib; - if (cinfo.out_color_components > 2) + if (mgr->cinfo.out_color_components > 2) ir = 0, ig = 1, ib = 2; - else if (cinfo.out_color_components > 1) + else if (mgr->cinfo.out_color_components > 1) ir = 0, ig = 1, ib = 0; else ir = 0, ig = 0, ib = 0; @@ -6437,15 +6482,15 @@ jpeg_load (struct frame *f, struct image *img) a default color, and we don't have to care about which colors can be freed safely, and which can't. */ init_color_table (); - colors = alloca (cinfo.actual_number_of_colors * sizeof *colors); + colors = alloca (mgr->cinfo.actual_number_of_colors * sizeof *colors); - for (i = 0; i < cinfo.actual_number_of_colors; ++i) + for (i = 0; i < mgr->cinfo.actual_number_of_colors; ++i) { /* Multiply RGB values with 255 because X expects RGB values in the range 0..0xffff. */ - int r = cinfo.colormap[ir][i] << 8; - int g = cinfo.colormap[ig][i] << 8; - int b = cinfo.colormap[ib][i] << 8; + int r = mgr->cinfo.colormap[ir][i] << 8; + int g = mgr->cinfo.colormap[ig][i] << 8; + int b = mgr->cinfo.colormap[ib][i] << 8; colors[i] = lookup_rgb_color (f, r, g, b); } @@ -6457,21 +6502,21 @@ jpeg_load (struct frame *f, struct image *img) } /* Read pixels. */ - row_stride = width * cinfo.output_components; - buffer = cinfo.mem->alloc_sarray ((j_common_ptr) &cinfo, JPOOL_IMAGE, - row_stride, 1); + row_stride = width * mgr->cinfo.output_components; + buffer = mgr->cinfo.mem->alloc_sarray ((j_common_ptr) &mgr->cinfo, + JPOOL_IMAGE, row_stride, 1); for (y = 0; y < height; ++y) { - fn_jpeg_read_scanlines (&cinfo, buffer, 1); - for (x = 0; x < cinfo.output_width; ++x) + fn_jpeg_read_scanlines (&mgr->cinfo, buffer, 1); + for (x = 0; x < mgr->cinfo.output_width; ++x) XPutPixel (ximg, x, y, colors[buffer[0][x]]); } /* Clean up. */ - fn_jpeg_finish_decompress (&cinfo); - fn_jpeg_destroy_decompress (&cinfo); + fn_jpeg_finish_decompress (&mgr->cinfo); + fn_jpeg_destroy_decompress (&mgr->cinfo); if (fp) - fclose ((FILE *) fp); + fclose (fp); /* Maybe fill in the background field while we have ximg handy. */ if (NILP (image_spec_value (img->spec, QCbackground, NULL))) @@ -6484,6 +6529,13 @@ jpeg_load (struct frame *f, struct image *img) return 1; } +static int +jpeg_load (struct frame *f, struct image *img) +{ + struct my_jpeg_error_mgr mgr; + return jpeg_load_body (f, img, &mgr); +} + #else /* HAVE_JPEG */ #ifdef HAVE_NS -- 2.20.1