problem report #100

problem report #100

[Dan Nicolaescu]

CID: 100
Checker: FORWARD_NULL (help)
File: base/src/emacs/src/xfaces.c
Function: realize_x_face
Description: Variable "default_face" tracked as NULL was dereferenced.


Event var_compare_op: Added "default_face" due to comparison "default_face != 0"
Also see events: [var_deref_op]
At conditional (1): "default_face != 0" taking false path

5853 	  if (default_face
5854 	      && lface_same_font_attributes_p (default_face->lface, attrs))
5855 	    {
5856 	      face->font = default_face->font;
5857 	      face->fontset
5858 		= make_fontset_for_ascii_face (f, default_face->fontset, face);
5859 	    }
5860 	  else
5861 	    {
5862 	      /* If the face attribute ATTRS specifies a fontset, use it as
5863 		 the base of a new realized fontset.  Otherwise, use the same
5864 		 base fontset as of the default face.  The base determines
5865 		 registry and encoding of a font.  It may also determine
5866 		 foundry and family.  The other fields of font name pattern
5867 		 are constructed from ATTRS.  */
5868 	      int fontset = face_fontset (attrs);
5869 	
5870 	      /* If we are realizing the default face, ATTRS should specify a
5871 		 fontset.  In other words, if FONTSET is -1, we are not
5872 		 realizing the default face, thus the default face should have
5873 		 already been realized.  */

At conditional (2): "fontset == -1" taking true path

5874 	      if (fontset == -1)

Event var_deref_op: Variable "default_face" tracked as NULL was dereferenced.
Also see events: [var_compare_op]

5875 		fontset = default_face->fontset;

Re: problem report #100

[Davis Herring]

> 5870 	      /* If we are realizing the default face, ATTRS should specify
> a
> 5871 		 fontset.  In other words, if FONTSET is -1, we are not
> 5872 		 realizing the default face, thus the default face should have
> 5873 		 already been realized.  */
>
> At conditional (2): "fontset == -1" taking true path
>
> 5874 	      if (fontset == -1)
>
> Event var_deref_op: Variable "default_face" tracked as NULL was
> dereferenced.
> Also see events: [var_compare_op]
>
> 5875 		fontset = default_face->fontset;

The comment says pretty clearly that this isn't a bug, but I'm not an
expert and can't guarantee that the default face always does get realized
first.  If anyone can verify that (should be trivial), no bug.

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.

Re: problem report #100

[Andreas Schwab]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> 5870 	      /* If we are realizing the default face, ATTRS should specify a
> 5871 		 fontset.  In other words, if FONTSET is -1, we are not
> 5872 		 realizing the default face, thus the default face should have
> 5873 		 already been realized.  */

The comment explains it already.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: problem report #100

[Chong Yidong]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> CID: 100
> Checker: FORWARD_NULL (help)
> File: base/src/emacs/src/xfaces.c
> Function: realize_x_face
> Description: Variable "default_face" tracked as NULL was dereferenced.

This was not really a bug, as the comments in xfaces.:5884 make clear.
However, I checked in a change that makes the abort condition more
precise, which should also prevent the checker from being confused.

Re: problem report #100

[Stefan Monnier]

> This was not really a bug, as the comments in xfaces.:5884 make clear.
> However, I checked in a change that makes the abort condition more
> precise, which should also prevent the checker from being confused.

This is indeed the best way to solve the non-bugs.
It's usually better to make the correctness obvious than to add
a comment explaining why this is correct.


        Stefan