Re: Build segfaults when loading image.el

[Dhruva Krishnamurthy]

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 <address@hidden>

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 <address@hidden> 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.