bug#8227: possibly uninitialized variables in update_window_fringes

bug#8227: possibly uninitialized variables in update_window_fringes

[Paul Eggert]

Severity: minor

I found this problem by compiling Emacs with GCC's -Wuninitialized flag.

The following code in the Emacs trunk src/fringe.c's
update_window_fringes function might be using uninitialized
variables:

   int top_row_ends_at_zv_p, bot_row_ends_at_zv_p;
   ...
   if (top_ind_rn >= 0)
     {
       ...
       top_row_ends_at_zv_p = row->ends_at_zv_p;
     }
   ...
   for (y = w->vscroll, rn = 0;
        y < yb && rn < nrows;
        y += row->height, rn++)
     {
       ...
       if (WINDOW_LEFT_FRINGE_WIDTH (w) == 0)
	...
       else if (row->left_user_fringe_bitmap != NO_FRINGE_BITMAP)
         ...
       else if ((!row->reversed_p && row->truncated_on_left_p)
	       || (row->reversed_p && row->truncated_on_right_p))
	...
       else if (row->indicate_bob_p && EQ (boundary_top, Qleft))
	{
	  left = ((row->indicate_eob_p && EQ (boundary_bot, Qleft))
		  ? LEFT_FRINGE (1, Qtop_bottom, top_row_ends_at_zv_p)
		  : LEFT_FRINGE (2, Qtop, 0));
           ...
	}

The last assignment uses top_row_ends_at_zv_p, but it's not clear
from the previous tests that top_row_ends_at_zv_p must be initialized.
There is a similar issue with bot_row_ends_at_zv_p.

I'm filing a bug report so that someone who is more expert in this
code can take a look at it.  In the meantime, I plan to work around
the problem by initializing the two local variables to 0, with a FIXME
explaining the situation: this shouldn't introduce a bug, because at
worst it will replace undefined behavior with defined behavior.

I'm CC'ing this to YAMAMOTO Mitsuharu, who committed the code in
question.

bug#8227: possibly uninitialized variables in update_window_fringes

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 10 Mar 2011 15:45:19 -0800, Paul Eggert <eggert@cs.ucla.edu> said:

> I found this problem by compiling Emacs with GCC's -Wuninitialized flag.

> The following code in the Emacs trunk src/fringe.c's
> update_window_fringes function might be using uninitialized
> variables:

>    int top_row_ends_at_zv_p, bot_row_ends_at_zv_p;
>    ...
>    if (top_ind_rn >= 0)
>      {
>        ...
>        top_row_ends_at_zv_p = row->ends_at_zv_p;
>      }
>    ...
>    for (y = w->vscroll, rn = 0;
>         y < yb && rn < nrows;
>         y += row->height, rn++)
>      {
>        ...
>        if (WINDOW_LEFT_FRINGE_WIDTH (w) == 0)
> 	...
>        else if (row->left_user_fringe_bitmap != NO_FRINGE_BITMAP)
>          ...
>        else if ((!row->reversed_p && row->truncated_on_left_p)
> 	       || (row->reversed_p && row->truncated_on_right_p))
> 	...
>        else if (row->indicate_bob_p && EQ (boundary_top, Qleft))
> 	{
> 	  left = ((row->indicate_eob_p && EQ (boundary_bot, Qleft))
> 		  ? LEFT_FRINGE (1, Qtop_bottom, top_row_ends_at_zv_p)
> 		  : LEFT_FRINGE (2, Qtop, 0));
>            ...
> 	}

> The last assignment uses top_row_ends_at_zv_p, but it's not clear
> from the previous tests that top_row_ends_at_zv_p must be initialized.
> There is a similar issue with bot_row_ends_at_zv_p.

No problem.  top_ind_rn is set to a non-negative value whenever
row->indicate_bob_p is set.

      for (y = w->vscroll, rn = 0;
	   y < yb && rn < nrows;
	   y += row->height, ++rn)
	{
		...

	  if (!row->mode_line_p)
	    {
	      if (top_ind_rn < 0 && row->visible_height > 0)
		{
		  if (MATRIX_ROW_START_CHARPOS (row) <= BUF_BEGV (XBUFFER (w->buffer))
		      && !MATRIX_ROW_PARTIALLY_VISIBLE_AT_TOP_P (w, row))
		    row->indicate_bob_p = !NILP (boundary_top);
		  else
		    row->indicate_top_line_p = !NILP (arrow_top);
		  top_ind_rn = rn;
		}

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#8227: possibly uninitialized variables in update_window_fringes

[Paul Eggert]

On 03/10/2011 05:31 PM, YAMAMOTO Mitsuharu wrote:

> No problem.  top_ind_rn is set to a non-negative value whenever
> row->indicate_bob_p is set.

Thanks for explaining that.  I plan to add the following comment
to the code, just before the declarations of
top_row_ends_at_zv_p and bot_row_ends_at_zv_p:

  /* top_ind_rn is set to a nonnegative value whenver                           
     row->indicate_bob_p is set, so it's OK that top_row_ends_at_zv_p           
     is not initialized here.  Similarly for bot_ind_rn,                        
     row->indicate_eob_p and bot_row_ends_at_zv_p.  */