Subject: Re: bug#12463: 24.2; pos-visible-in-window-p gets slower over time

From: Eli Zaretskii <[email-masked]>
Date: Fri, 21 Sep 2012 20:03:34 +0300

> From: Chong Yidong <[email-masked]>
> Date: Fri, 21 Sep 2012 18:01:19 +0800
> Cc: Jörg Walter <[email-masked]>, [email-masked]
> 
> Thanks.  Fixed in the attached patch, which also addresses Eli's other
> comments.  Please take a look again.

Thanks.

>   /* Define a new image type from TYPE.  This adds a copy of TYPE to
> !    image_types and caches the loading status of TYPE.
>   
> !    LIBRARIES is an alist associating dynamic libraries to external
> !    files implementing them, which is passed to the image library
> !    initialization function if necessary.  A nil value defaults to
> !    Vdynamic_library_alist.  */
> ! 
> ! static struct image_type *
> ! define_image_type (struct image_type *type, Lisp_Object libraries)
>   {
> !   struct image_type *p = NULL;
> !   Lisp_Object target_type = *type->type;
> !   int type_valid = 1;
>   
> !   for (p = image_types; p; p = p->next)
> !     if (EQ (*p->type, target_type))
> !       return p;
> ! 
> !   if (type->init)
>       {
> ! #ifdef HAVE_NTGUI
> !       /* If we failed to load the library before, don't try again.  */
> !       Lisp_Object tested = Fassq (target_type, Vlibrary_cache);
> !       if (CONSP (tested) && NILP (XCDR (tested)))
> ! 	type_valid = 0;
> !       else
> ! #endif
> ! 	{
> ! 	  /* If the load failed, avoid trying again.  */
> ! 	  type_valid = (*type->init)(libraries);
> ! 	  CACHE_IMAGE_TYPE (target_type, type_valid);
> ! 	}
> !     }
>   
> +   if (type_valid)
> +     {
>         /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
>            The initialized data segment is read-only.  */
>         p = xmalloc (sizeof *p);
>         *p = *type;
>         p->next = image_types;
>         image_types = p;
>       }
>   
> !   return p;
>   }

Am I missing something, or will this really call the initialization
function again and again and again?  AFAIU, if an image's library is
already in Vlibrary_cache, and the cdr of its association is non-nil,
that means the initialization function was already called for the
corresponding image type, and it returned non-zero.  So there's no
need to call it again.