Re: master updated (05b7953 -> b2b1ccb)

Re: master updated (05b7953 -> b2b1ccb)

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> This patch series seems to introduce the following warnings?
>
> xdisp.c: In function ‘normal_char_ascent_descent’:
> xdisp.c:26433:13: warning: potential null pointer dereference [-Wnull-dereference]
>     if (!(pcm->width == 0 && pcm->rbearing == 0 && pcm->lbearing == 0))
>           ~~~^~~~~~~
> xdisp.c: In function ‘gui_produce_glyphs’:
> xdisp.c:28482:15: warning: potential null pointer dereference [-Wnull-dereference]
>         if (pcm->width == 0
>             ~~~^~~~~~~
> xdisp.c:28745:16: warning: potential null pointer dereference [-Wnull-dereference]
>          if (pcm->width == 0
>              ~~~^~~~~~~

Hmm, this is weird: the warnings seem to be legitimate
(get_per_char_metric can return NULL), but I don't see how my patch
would _introduce_ these warnings.

Also, I don't get these warnings; what's your compiler version? If it's
GCC 9, perhaps the warning detection has improved. I don't suppose you
happened to update your compiler at the same time as updating your local
branch?

It's good to plug these anyway, but it would be nice to figure out why
they're only popping up now.

The below diff should silence the warnings.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pcm --]
[-- Type: text/x-patch, Size: 1496 bytes --]

diff --git a/src/xdisp.c b/src/xdisp.c
index c561ea9e36..ca95f8f944 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -26430,7 +26430,10 @@ normal_char_ascent_descent (struct font *font, int c, int *ascent, int *descent)
 	{
 	  struct font_metrics *pcm = get_per_char_metric (font, &char2b);
 
-	  if (!(pcm->width == 0 && pcm->rbearing == 0 && pcm->lbearing == 0))
+	  if (!(pcm
+                && pcm->width    == 0
+                && pcm->rbearing == 0
+                && pcm->lbearing == 0))
 	    {
 	      /* We add 1 pixel to character dimensions as heuristics
 		 that produces nicer display, e.g. when the face has
@@ -28479,8 +28482,10 @@ gui_produce_glyphs (struct it *it)
 	  if (get_char_glyph_code (it->char_to_display, font, &char2b))
 	    {
 	      pcm = get_per_char_metric (font, &char2b);
-	      if (pcm->width == 0
-		  && pcm->rbearing == 0 && pcm->lbearing == 0)
+	      if (pcm
+                  && pcm->width    == 0
+                  && pcm->rbearing == 0
+                  && pcm->lbearing == 0)
 		pcm = NULL;
 	    }
 
@@ -28742,8 +28747,10 @@ gui_produce_glyphs (struct it *it)
 		  if (get_char_glyph_code (' ', font, &char2b))
 		    {
 		      pcm = get_per_char_metric (font, &char2b);
-		      if (pcm->width == 0
-			  && pcm->rbearing == 0 && pcm->lbearing == 0)
+		      if (pcm
+                          && pcm->width    == 0
+                          && pcm->rbearing == 0
+                          && pcm->lbearing == 0)
 			pcm = NULL;
 		    }
 

Re: master updated (05b7953 -> b2b1ccb)

[Lars Ingebrigtsen]

Alex Gramiak <agrambot@gmail.com> writes:

> Hmm, this is weird: the warnings seem to be legitimate
> (get_per_char_metric can return NULL), but I don't see how my patch
> would _introduce_ these warnings.

Hm, perhaps I was wrong that your patches introduced the warnings -- I
didn't try reverting and checking, but I couldn't recall seeing them
earlier...

> Also, I don't get these warnings; what's your compiler version? If it's
> GCC 9, perhaps the warning detection has improved. I don't suppose you
> happened to update your compiler at the same time as updating your local
> branch?

Old gcc:

[larsi@stories ~/src/emacs/trunk]$ gcc --version
gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
Copyright (C) 2016 Free Software Foundation, Inc.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master updated (05b7953 -> b2b1ccb)

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

Alex Gramiak <agrambot@gmail.com> writes:

> The below diff should silence the warnings.
>
> diff --git a/src/xdisp.c b/src/xdisp.c
> index c561ea9e36..ca95f8f944 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -26430,7 +26430,10 @@ normal_char_ascent_descent (struct font *font, int c, int *ascent, int *descent)
>  	{
>  	  struct font_metrics *pcm = get_per_char_metric (font, &char2b);
>  
> -	  if (!(pcm->width == 0 && pcm->rbearing == 0 && pcm->lbearing == 0))
> +	  if (!(pcm
> +                && pcm->width    == 0
> +                && pcm->rbearing == 0
> +                && pcm->lbearing == 0))
>  	    {
>  	      /* We add 1 pixel to character dimensions as heuristics
>  		 that produces nicer display, e.g. when the face has

Whoops, the pcm check shouldn't happen inside the negation.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pcm v2 --]
[-- Type: text/x-patch, Size: 1492 bytes --]

diff --git a/src/xdisp.c b/src/xdisp.c
index c561ea9e36..2d417be03e 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -26430,7 +26430,9 @@ normal_char_ascent_descent (struct font *font, int c, int *ascent, int *descent)
 	{
 	  struct font_metrics *pcm = get_per_char_metric (font, &char2b);
 
-	  if (!(pcm->width == 0 && pcm->rbearing == 0 && pcm->lbearing == 0))
+	  if (pcm && !(pcm->width    == 0
+                       && pcm->rbearing == 0
+                       && pcm->lbearing == 0))
 	    {
 	      /* We add 1 pixel to character dimensions as heuristics
 		 that produces nicer display, e.g. when the face has
@@ -28479,8 +28481,10 @@ gui_produce_glyphs (struct it *it)
 	  if (get_char_glyph_code (it->char_to_display, font, &char2b))
 	    {
 	      pcm = get_per_char_metric (font, &char2b);
-	      if (pcm->width == 0
-		  && pcm->rbearing == 0 && pcm->lbearing == 0)
+	      if (pcm
+                  && pcm->width    == 0
+                  && pcm->rbearing == 0
+                  && pcm->lbearing == 0)
 		pcm = NULL;
 	    }
 
@@ -28742,8 +28746,10 @@ gui_produce_glyphs (struct it *it)
 		  if (get_char_glyph_code (' ', font, &char2b))
 		    {
 		      pcm = get_per_char_metric (font, &char2b);
-		      if (pcm->width == 0
-			  && pcm->rbearing == 0 && pcm->lbearing == 0)
+		      if (pcm
+                          && pcm->width    == 0
+                          && pcm->rbearing == 0
+                          && pcm->lbearing == 0)
 			pcm = NULL;
 		    }
 

Re: master updated (05b7953 -> b2b1ccb)

[Paul Eggert]

On 5/20/19 11:47 AM, Alex Gramiak wrote:
> Hmm, this is weird: the warnings seem to be legitimate
> (get_per_char_metric can return NULL)

No, the warnings are not legitimate. Those calls to get_per_char_metric 
occur only when get_char_glyph_code succeeds, and when that happens 
get_per_char_metric cannot fail.

I wouldn't worry about bogus warnings issued by older instances of GCC, 
or issued by GCC with non-default build options. There are too many such 
warnings, and we'd waste time (and clutter up the code, perhaps 
incorrectly) pacifying them. Just worry about the latest GCC with 
default build options.