[Emacs-diffs] /srv/bzr/emacs/trunk r110001: More fixes for 'volatile' and setjmp/longjmp.

[Paul Eggert]

------------------------------------------------------------
revno: 110001
committer: Paul Eggert <address@hidden>
branch nick: trunk
timestamp: Wed 2012-09-12 14:34:24 -0700
message:
  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.
modified:
  src/ChangeLog
  src/eval.c
  src/image.c

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2012-09-12 19:16:36 +0000
+++ b/src/ChangeLog     2012-09-12 21:34:24 +0000
@@ -1,3 +1,19 @@
+2012-09-12  Paul Eggert  <address@hidden>
+
+       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  <address@hidden>
 
        * keyboard.c (read_char, requeued_events_pending_p, Finput_pending_p)

=== modified file 'src/eval.c'
--- a/src/eval.c        2012-09-12 00:14:50 +0000
+++ b/src/eval.c        2012-09-12 21:34:24 +0000
@@ -707,7 +707,7 @@
       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 @@
 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);
 }

=== modified file 'src/image.c'
--- a/src/image.c       2012-09-12 14:10:42 +0000
+++ b/src/image.c       2012-09-12 21:34:24 +0000
@@ -5591,20 +5591,31 @@
 /* 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_ptr = fn_png_create_read_struct (PNG_LIBPNG_VER_STRING,
                                       NULL, my_png_error,
                                       my_png_warning);
-  if (!png_ptr)
-    {
-      if (fp) fclose (fp);
-      return 0;
-    }
-
-  info_ptr = fn_png_create_info_struct (png_ptr);
-  if (!info_ptr)
-    {
-      fn_png_destroy_read_struct (&png_ptr, NULL, NULL);
-      if (fp) fclose (fp);
-      return 0;
-    }
-
-  end_info = fn_png_create_info_struct (png_ptr);
-  if (!end_info)
-    {
-      fn_png_destroy_read_struct (&png_ptr, &info_ptr, NULL);
+  if (png_ptr)
+    {
+      info_ptr = fn_png_create_info_struct (png_ptr);
+      end_info = fn_png_create_info_struct (png_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 (&c->png_ptr, &c->info_ptr, &c->end_info);
+      png_ptr = 0;
+    }
+  if (! png_ptr)
+    {
       if (fp) fclose (fp);
       return 0;
     }
@@ -5698,14 +5711,18 @@
   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 @@
   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 @@
   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 @@
 #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 @@
   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 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 @@
 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 @@
    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 @@
 
   /* 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 @@
 
   /* 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 @@
        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 @@
   }
 
   /* 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 @@
   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