Re: Debugging emacs memory management

[Eli Zaretskii <eliz@gnu.org>]

> From: Dima Kogan <lists@dima.secretsauce.net>
> Cc: schwab@suse.de, eggert@cs.ucla.edu, emacs-devel@gnu.org
> Date: Mon, 05 Oct 2015 11:19:14 -0700
> 
> >> > This better be always true, otherwise XCAR (XCAR (tail)) would be a bug.
> >> 
> >> It isn't. Without that check I was getting every time.
> >> 
> >>   *ERROR*: Wrong type argument: listp, "monospace-10"
> >
> > I think what Andreas meant was this: the old code used
> > XCAR (XCAR (tail)) unconditionally, which seems to indicate that
> > XCAR (tail) is always a cons cell, and your test should always
> > yield true.
> 
> I just looked at it again, and it was my mistake; this is always a cons
> cell as it should be. I was getting the error when using Fcopy_sequence
> instead of making a new cell with Fcons. The former would make a deeper
> copy, but the alist entries aren't lists, so Fcopy_sequence() was
> barfing. Making a new cons-cell is one-level deep, and it's deep-enough,
> so it works. So yeah, my bad.
> 
> 
> >> Surprised me too, and I haven't checked to see what was happening there.
> >
> > So maybe your code should include some check for QCfont_entity in the
> > 'else' clause as well.
> 
> Here's the updated patch:
> 
> diff --git a/src/font.c b/src/font.c
> index 8e06532..dd574ca 100644
> --- a/src/font.c
> +++ b/src/font.c
> @@ -3981,7 +3981,12 @@ copy_font_spec (Lisp_Object font)
>    pcdr = spec->props + FONT_EXTRA_INDEX;
>    for (tail = AREF (font, FONT_EXTRA_INDEX); CONSP (tail); tail = XCDR (tail))
>      if (!EQ (XCAR (XCAR (tail)), QCfont_entity))
> -      *pcdr = Fcons (XCAR (tail), Qnil), pcdr = xcdr_addr (*pcdr);
> +      {
> +        *pcdr = Fcons (Fcons( XCAR (XCAR (tail)),
> +                              XCDR (XCAR (tail))),
> +                       Qnil);
> +        pcdr = xcdr_addr (*pcdr);
> +      }
>  
>    XSETFONT (new_spec, spec);
>    return new_spec;
> 
> I'm not sure what you mean about checking for QCfont_entity. This new
> patch doesn't add an if(), and there's already a QCfont_entity check
> there. This is fine, right?

Forget that.  It was based on your reports about errors.