Re: Build segfaults when loading image.el

[Dhruva Krishnamurthy <dhruvakm@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2219 bytes --]

On further reading the ImageMagick source, I would that the calls to
'MagickWandGenesis' protect themselves against redundant repeat calls.
Hence, there is no need to protect it using a static variable. The
following patch is a minimal fix.

Link to source:
https://github.com/ImageMagick/ImageMagick/blob/f775a5cf27a95c42bb6d19b50f4869db265fdaa9/MagickWand/magick-wand.c#L1001

-dhruva

commit 23236f26d2f5839f2fdb7022039dcf102dcf3a3b
Author: Dhruva Krishnamurthy <dhruvakm@gmail.com>
Date:   Mon Dec 23 14:49:51 2019 -0800

    Initialize ImageMagick runtime environment before use

diff --git a/src/image.c b/src/image.c
index 70d932f9ed..75c31cad57 100644
--- a/src/image.c
+++ b/src/image.c
@@ -8875,13 +8875,8 @@ imagemagick_load_image (struct frame *f, struct
image *img,
   char hint_buffer[MaxTextExtent];
   char *filename_hint = NULL;

-  /* Initialize the ImageMagick environment.  */
-  static bool imagemagick_initialized;
-  if (!imagemagick_initialized)
-    {
-      imagemagick_initialized = true;
-      MagickWandGenesis ();
-    }
+  /* Initialize the ImageMagick environment if not initialized. */
+  MagickWandGenesis ();

   /* Handle image index for image types who can contain more than one
image.
      Interface :index is same as for GIF.  First we "ping" the image to
see how
@@ -9290,6 +9285,9 @@ DEFUN ("imagemagick-types", Fimagemagick_types,
Simagemagick_types, 0, 0, 0,
   char **imtypes;
   size_t i;

+  /* Initialize the ImageMagick environment if not initialized. */
+  MagickWandGenesis ();
+
   ex = AcquireExceptionInfo ();
   imtypes = GetMagickList ("*", &numf, ex);
   DestroyExceptionInfo (ex);


On Mon, Dec 23, 2019 at 6:51 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> On 12/23/19 6:09 PM, Dhruva Krishnamurthy wrote:
> > I still feel the original patch calling into IsMagickWandInstantiated()
> was
> > more correct
>
> Not sure I follow, but anyway any change along those lines would be more
> experimental, whereas the emacs-27 branch is for bug-fixing and is
> maintained
> more conservatively.
>
> PS. I didn't notice the "!!IsMagickWandInstantiated();" part of your
> patch; if I
> had, I would have omitted it from the emacs-27 branch and would have said
> why.
>

[-- Attachment #2: Type: text/html, Size: 4603 bytes --]