Re: Problem report #16

["Stuart D. Herring" <herring@lanl.gov>]

There's actually several possible problems here.

>   > At conditional (1): "face != 0" taking false path
>   >
>   > 667  	  if (face)
>   > 668  	    id = face->fontset;

This is a red herring -- face is reassigned later.

>   > At conditional (2): "id < 0" taking true path
>   >
>   > 669  	  if (id < 0)
>   > 670  	    fontset = Qnil;
>   > 671  	  else
>   > 672  	    fontset = FONTSET_FROM_ID (id);

Here's one thing I don't like: FONTSET_FROM_ID doesn't do any safety
checks on id.  Can we trust this function to always get reasonable id
values?

>   > 682  		  int face_id = XINT (elt);
>   > 684  		  xassert (face_id == face->id);

Dan wrote (although not here in his message):
> This problem could happen if fs_load_font was called with face=NULL
> and id>0. Can that happen?

Here's where Dan's point is relevant: if face==NULL and id>0, then it
seems quite possible for this line to be reached...

>   > 685  		  face = FACE_FROM_ID (f, face_id);

...just before assigning face, so it'd still be NULL.  But there's more:
FACE_FROM_ID can fail and return NULL:

>   > Event var_deref_op: Variable "face" tracked as NULL was dereferenced.
>   > Also see events: [var_compare_op]
>   >
>   > 686  		  return (*get_font_info_func) (f, face->font_info_id);

So this part is dangerous if and only if face_id, derived from the
fontset, can be messed up.  Nothing to do with face's value at entry to
the function.  Someone who understands fontsets/font loading, comment?

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.