vim-patch:8.1.0623: iterating through window frames is repeated

Problem:    Iterating through window frames is repeated.
Solution:   Define FOR_ALL_FRAMES. (Yegappan Lakshmanan)
3d1491ed23

src/nvim/ex_docmd.c

@@ -9323,26 +9323,30 @@ static frame_T *ses_skipframe(frame_T *fr)
 {
   frame_T     *frc;
 
-  for (frc = fr; frc != NULL; frc = frc->fr_next)
+  FOR_ALL_FRAMES(frc, fr) {
-    if (ses_do_frame(frc))
+    if (ses_do_frame(frc)) {
       break;
+    }
+  }
   return frc;
 }
 
-/*
+// Return true if frame "fr" has a window somewhere that we want to save in
- * Return TRUE if frame "fr" has a window somewhere that we want to save in
+// the Session.
- * the Session.
+static bool ses_do_frame(const frame_T *fr)
- */
+  FUNC_ATTR_NONNULL_ARG(1)
-static int ses_do_frame(frame_T *fr)
 {
-  frame_T     *frc;
+  const frame_T *frc;
 
-  if (fr->fr_layout == FR_LEAF)
+  if (fr->fr_layout == FR_LEAF) {
     return ses_do_win(fr->fr_win);
-  for (frc = fr->fr_child; frc != NULL; frc = frc->fr_next)
+  }
-    if (ses_do_frame(frc))
+  FOR_ALL_FRAMES(frc, fr->fr_child) {
-      return TRUE;
+    if (ses_do_frame(frc)) {
-  return FALSE;
+      return true;
+    }
+  }
+  return false;
 }
 
 /// Return non-zero if window "wp" is to be stored in the Session.

src/nvim/globals.h

@@ -435,10 +435,11 @@ EXTERN win_T    *firstwin;              /* first window */
 EXTERN win_T    *lastwin;               /* last window */
 EXTERN win_T    *prevwin INIT(= NULL);  /* previous window */
 # define ONE_WINDOW (firstwin == lastwin)
-/*
+# define FOR_ALL_FRAMES(frp, first_frame) \
- * When using this macro "break" only breaks out of the inner loop. Use "goto"
+  for (frp = first_frame; frp != NULL; frp = frp->fr_next)  // NOLINT
- * to break out of the tabpage loop.
+
- */
+// When using this macro "break" only breaks out of the inner loop. Use "goto"
+// to break out of the tabpage loop.
 # define FOR_ALL_TAB_WINDOWS(tp, wp) \
   FOR_ALL_TABS(tp) \
     FOR_ALL_WINDOWS_IN_TAB(wp, tp)

src/nvim/screen.c

@@ -4571,17 +4571,21 @@ void redraw_statuslines(void)
 /*
  * Redraw all status lines at the bottom of frame "frp".
  */
-void win_redraw_last_status(frame_T *frp)
+void win_redraw_last_status(const frame_T *frp)
+  FUNC_ATTR_NONNULL_ARG(1)
 {
-  if (frp->fr_layout == FR_LEAF)
+  if (frp->fr_layout == FR_LEAF) {
-    frp->fr_win->w_redr_status = TRUE;
+    frp->fr_win->w_redr_status = true;
-  else if (frp->fr_layout == FR_ROW) {
+  } else if (frp->fr_layout == FR_ROW) {
-    for (frp = frp->fr_child; frp != NULL; frp = frp->fr_next)
+    FOR_ALL_FRAMES(frp, frp->fr_child) {
       win_redraw_last_status(frp);
-  } else { /* frp->fr_layout == FR_COL */
+    }
+  } else {
+    assert(frp->fr_layout == FR_COL);
     frp = frp->fr_child;
-    while (frp->fr_next != NULL)
+    while (frp->fr_next != NULL) {
       frp = frp->fr_next;
+    }
     win_redraw_last_status(frp);
   }
 }

src/nvim/window.c

@@ -1024,7 +1024,7 @@ int win_split_ins(int size, int flags, win_T *new_wp, int dir)
       for (frp = oldwin->w_frame->fr_parent; frp != NULL;
            frp = frp->fr_parent) {
         if (frp->fr_layout == FR_ROW) {
-          for (frp2 = frp->fr_child; frp2 != NULL; frp2 = frp2->fr_next) {
+          FOR_ALL_FRAMES(frp2, frp->fr_child) {
             if (frp2 != prevfrp) {
               minwidth += frame_minwidth(frp2, NOWIN);
             }
@@ -1102,7 +1102,7 @@ int win_split_ins(int size, int flags, win_T *new_wp, int dir)
       for (frp = oldwin->w_frame->fr_parent; frp != NULL;
            frp = frp->fr_parent) {
         if (frp->fr_layout == FR_COL) {
-          for (frp2 = frp->fr_child; frp2 != NULL; frp2 = frp2->fr_next) {
+          FOR_ALL_FRAMES(frp2, frp->fr_child) {
             if (frp2 != prevfrp) {
               minheight += frame_minheight(frp2, NOWIN);
             }
@@ -1247,12 +1247,14 @@ int win_split_ins(int size, int flags, win_T *new_wp, int dir)
     curfrp->fr_child = frp;
     curfrp->fr_win = NULL;
     curfrp = frp;
-    if (frp->fr_win != NULL)
+    if (frp->fr_win != NULL) {
       oldwin->w_frame = frp;
-    else
+    } else {
-      for (frp = frp->fr_child; frp != NULL; frp = frp->fr_next)
+      FOR_ALL_FRAMES(frp, frp->fr_child) {
         frp->fr_parent = curfrp;
       }
+    }
+  }
 
   if (new_wp == NULL)
     frp = wp->w_frame;
@@ -1717,13 +1719,13 @@ static void win_rotate(bool upwards, int count)
     return;
   }
 
-  /* Check if all frames in this row/col have one window. */
+  // Check if all frames in this row/col have one window.
-  for (frp = curwin->w_frame->fr_parent->fr_child; frp != NULL;
+  FOR_ALL_FRAMES(frp, curwin->w_frame->fr_parent->fr_child) {
-       frp = frp->fr_next)
     if (frp->fr_win == NULL) {
       EMSG(_("E443: Cannot rotate when another window is split"));
       return;
     }
+  }
 
   while (count--) {
     if (upwards) {              /* first window becomes last window */
@@ -1961,10 +1963,10 @@ static void win_equal_rec(
         room = 0;
       } else {
         next_curwin_size = -1;
-        for (fr = topfr->fr_child; fr != NULL; fr = fr->fr_next) {
+        FOR_ALL_FRAMES(fr, topfr->fr_child) {
-          /* If 'winfixwidth' set keep the window width if
+          // If 'winfixwidth' set keep the window width if
-           * possible.
+          // possible.
-           * Watch out for this window being the next_curwin. */
+          // Watch out for this window being the next_curwin.
           if (!frame_fixed_width(fr)) {
             continue;
           }
@@ -2007,7 +2009,7 @@ static void win_equal_rec(
         --totwincount;                  /* don't count curwin */
     }
 
-    for (fr = topfr->fr_child; fr != NULL; fr = fr->fr_next) {
+    FOR_ALL_FRAMES(fr, topfr->fr_child) {
       wincount = 1;
       if (fr->fr_next == NULL)
         /* last frame gets all that remains (avoid roundoff error) */
@@ -2082,10 +2084,10 @@ static void win_equal_rec(
         room = 0;
       } else {
         next_curwin_size = -1;
-        for (fr = topfr->fr_child; fr != NULL; fr = fr->fr_next) {
+        FOR_ALL_FRAMES(fr, topfr->fr_child) {
-          /* If 'winfixheight' set keep the window height if
+          // If 'winfixheight' set keep the window height if
-           * possible.
+          // possible.
-           * Watch out for this window being the next_curwin. */
+          // Watch out for this window being the next_curwin.
           if (!frame_fixed_height(fr)) {
             continue;
           }
@@ -2128,7 +2130,7 @@ static void win_equal_rec(
         --totwincount;                  /* don't count curwin */
     }
 
-    for (fr = topfr->fr_child; fr != NULL; fr = fr->fr_next) {
+    FOR_ALL_FRAMES(fr, topfr->fr_child) {
       wincount = 1;
       if (fr->fr_next == NULL)
         /* last frame gets all that remains (avoid roundoff error) */
@@ -2815,8 +2817,9 @@ winframe_remove (
      * and remove it. */
     frp2->fr_parent->fr_layout = frp2->fr_layout;
     frp2->fr_parent->fr_child = frp2->fr_child;
-    for (frp = frp2->fr_child; frp != NULL; frp = frp->fr_next)
+    FOR_ALL_FRAMES(frp, frp2->fr_child) {
       frp->fr_parent = frp2->fr_parent;
+    }
     frp2->fr_parent->fr_win = frp2->fr_win;
     if (frp2->fr_win != NULL)
       frp2->fr_win->w_frame = frp2->fr_parent;
@@ -2937,13 +2940,14 @@ static win_T *frame2win(frame_T *frp)
 ///
 /// @param  frp  frame
 /// @param  wp   window
-static bool frame_has_win(frame_T *frp, win_T *wp)
+static bool frame_has_win(const frame_T *frp, const win_T *wp)
   FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ARG(1)
 {
   if (frp->fr_layout == FR_LEAF) {
     return frp->fr_win == wp;
   }
-  for (frame_T *p = frp->fr_child; p != NULL; p = p->fr_next) {
+  const frame_T *p;
+  FOR_ALL_FRAMES(p, frp->fr_child) {
     if (frame_has_win(p, wp)) {
       return true;
     }
@@ -2974,8 +2978,8 @@ frame_new_height (
         height - topfrp->fr_win->w_status_height);
   } else if (topfrp->fr_layout == FR_ROW) {
     do {
-      /* All frames in this row get the same new height. */
+      // All frames in this row get the same new height.
-      for (frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next) {
+      FOR_ALL_FRAMES(frp, topfrp->fr_child) {
         frame_new_height(frp, height, topfirst, wfh);
         if (frp->fr_height > height) {
           /* Could not fit the windows, make the whole row higher. */
@@ -3056,7 +3060,7 @@ static bool frame_fixed_height(frame_T *frp)
   if (frp->fr_layout == FR_ROW) {
     // The frame is fixed height if one of the frames in the row is fixed
     // height.
-    for (frp = frp->fr_child; frp != NULL; frp = frp->fr_next) {
+    FOR_ALL_FRAMES(frp, frp->fr_child) {
       if (frame_fixed_height(frp)) {
         return true;
       }
@@ -3066,7 +3070,7 @@ static bool frame_fixed_height(frame_T *frp)
 
   // frp->fr_layout == FR_COL: The frame is fixed height if all of the
   // frames in the row are fixed height.
-  for (frp = frp->fr_child; frp != NULL; frp = frp->fr_next) {
+  FOR_ALL_FRAMES(frp, frp->fr_child) {
     if (!frame_fixed_height(frp)) {
       return false;
     }
@@ -3090,7 +3094,7 @@ static bool frame_fixed_width(frame_T *frp)
   if (frp->fr_layout == FR_COL) {
     // The frame is fixed width if one of the frames in the row is fixed
     // width.
-    for (frp = frp->fr_child; frp != NULL; frp = frp->fr_next) {
+    FOR_ALL_FRAMES(frp, frp->fr_child) {
       if (frame_fixed_width(frp)) {
         return true;
       }
@@ -3100,7 +3104,7 @@ static bool frame_fixed_width(frame_T *frp)
 
   // frp->fr_layout == FR_ROW: The frame is fixed width if all of the
   // frames in the row are fixed width.
-  for (frp = frp->fr_child; frp != NULL; frp = frp->fr_next) {
+  FOR_ALL_FRAMES(frp, frp->fr_child) {
     if (!frame_fixed_width(frp)) {
       return false;
     }
@@ -3124,13 +3128,15 @@ static void frame_add_statusline(frame_T *frp)
       wp->w_status_height = STATUS_HEIGHT;
     }
   } else if (frp->fr_layout == FR_ROW) {
-    /* Handle all the frames in the row. */
+    // Handle all the frames in the row.
-    for (frp = frp->fr_child; frp != NULL; frp = frp->fr_next)
+    FOR_ALL_FRAMES(frp, frp->fr_child) {
       frame_add_statusline(frp);
-  } else { /* frp->fr_layout == FR_COL */
+    }
-          /* Only need to handle the last frame in the column. */
+  } else {
-    for (frp = frp->fr_child; frp->fr_next != NULL; frp = frp->fr_next)
+    assert(frp->fr_layout == FR_COL);
-      ;
+    // Only need to handle the last frame in the column.
+    for (frp = frp->fr_child; frp->fr_next != NULL; frp = frp->fr_next) {
+    }
     frame_add_statusline(frp);
   }
 }
@@ -3165,8 +3171,8 @@ frame_new_width (
     win_new_width(wp, width - wp->w_vsep_width);
   } else if (topfrp->fr_layout == FR_COL) {
     do {
-      /* All frames in this column get the same new width. */
+      // All frames in this column get the same new width.
-      for (frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next) {
+      FOR_ALL_FRAMES(frp, topfrp->fr_child) {
         frame_new_width(frp, width, leftfirst, wfw);
         if (frp->fr_width > width) {
           /* Could not fit the windows, make whole column wider. */
@@ -3235,7 +3241,8 @@ frame_new_width (
  * Add the vertical separator to windows at the right side of "frp".
  * Note: Does not check if there is room!
  */
-static void frame_add_vsep(frame_T *frp)
+static void frame_add_vsep(const frame_T *frp)
+  FUNC_ATTR_NONNULL_ARG(1)
 {
   win_T       *wp;
 
@@ -3247,11 +3254,13 @@ static void frame_add_vsep(frame_T *frp)
       wp->w_vsep_width = 1;
     }
   } else if (frp->fr_layout == FR_COL) {
-    /* Handle all the frames in the column. */
+    // Handle all the frames in the column.
-    for (frp = frp->fr_child; frp != NULL; frp = frp->fr_next)
+    FOR_ALL_FRAMES(frp, frp->fr_child) {
       frame_add_vsep(frp);
-  } else { /* frp->fr_layout == FR_ROW */
+    }
-          /* Only need to handle the last frame in the row. */
+  } else {
+    assert(frp->fr_layout == FR_ROW);
+    // Only need to handle the last frame in the row.
     frp = frp->fr_child;
     while (frp->fr_next != NULL)
       frp = frp->fr_next;
@@ -3301,7 +3310,7 @@ static int frame_minheight(frame_T *topfrp, win_T *next_curwin)
   } else if (topfrp->fr_layout == FR_ROW) {
     /* get the minimal height from each frame in this row */
     m = 0;
-    for (frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next) {
+    FOR_ALL_FRAMES(frp, topfrp->fr_child) {
       n = frame_minheight(frp, next_curwin);
       if (n > m)
         m = n;
@@ -3309,9 +3318,10 @@ static int frame_minheight(frame_T *topfrp, win_T *next_curwin)
   } else {
     /* Add up the minimal heights for all frames in this column. */
     m = 0;
-    for (frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next)
+    FOR_ALL_FRAMES(frp, topfrp->fr_child) {
       m += frame_minheight(frp, next_curwin);
     }
+  }
 
   return m;
 }
@@ -3344,7 +3354,7 @@ frame_minwidth (
   } else if (topfrp->fr_layout == FR_COL) {
     /* get the minimal width from each frame in this column */
     m = 0;
-    for (frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next) {
+    FOR_ALL_FRAMES(frp, topfrp->fr_child) {
       n = frame_minwidth(frp, next_curwin);
       if (n > m)
         m = n;
@@ -3352,9 +3362,10 @@ frame_minwidth (
   } else {
     /* Add up the minimal widths for all frames in this row. */
     m = 0;
-    for (frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next)
+    FOR_ALL_FRAMES(frp, topfrp->fr_child) {
       m += frame_minwidth(frp, next_curwin);
     }
+  }
 
   return m;
 }
@@ -4830,11 +4841,12 @@ static void frame_comp_pos(frame_T *topfrp, int *row, int *col)
   } else {
     startrow = *row;
     startcol = *col;
-    for (frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next) {
+    FOR_ALL_FRAMES(frp, topfrp->fr_child) {
-      if (topfrp->fr_layout == FR_ROW)
+      if (topfrp->fr_layout == FR_ROW) {
-        *row = startrow;                /* all frames are at the same row */
+        *row = startrow;  // all frames are at the same row
-      else
+      } else {
-        *col = startcol;                /* all frames are at the same col */
+        *col = startcol;  // all frames are at the same col
+      }
       frame_comp_pos(frp, row, col);
     }
   }
@@ -4943,16 +4955,17 @@ static void frame_setheight(frame_T *curfrp, int height)
     for (run = 1; run <= 2; ++run) {
       room = 0;
       room_reserved = 0;
-      for (frp = curfrp->fr_parent->fr_child; frp != NULL;
+      FOR_ALL_FRAMES(frp, curfrp->fr_parent->fr_child) {
-           frp = frp->fr_next) {
         if (frp != curfrp
             && frp->fr_win != NULL
-            && frp->fr_win->w_p_wfh)
+            && frp->fr_win->w_p_wfh) {
           room_reserved += frp->fr_height;
+        }
         room += frp->fr_height;
-        if (frp != curfrp)
+        if (frp != curfrp) {
           room -= frame_minheight(frp, NULL);
         }
+      }
       if (curfrp->fr_width != Columns) {
         room_cmdline = 0;
       } else {
@@ -5124,16 +5137,17 @@ static void frame_setwidth(frame_T *curfrp, int width)
     for (run = 1; run <= 2; ++run) {
       room = 0;
       room_reserved = 0;
-      for (frp = curfrp->fr_parent->fr_child; frp != NULL;
+      FOR_ALL_FRAMES(frp, curfrp->fr_parent->fr_child) {
-           frp = frp->fr_next) {
         if (frp != curfrp
             && frp->fr_win != NULL
-            && frp->fr_win->w_p_wfw)
+            && frp->fr_win->w_p_wfw) {
           room_reserved += frp->fr_width;
+        }
         room += frp->fr_width;
-        if (frp != curfrp)
+        if (frp != curfrp) {
           room -= frame_minwidth(frp, NULL);
         }
+      }
 
       if (width <= room)
         break;
@@ -5296,10 +5310,11 @@ void win_drag_status_line(win_T *dragwin, int offset)
       room -= p_ch;
     if (room < 0)
       room = 0;
-    /* sum up the room of frames below of the current one */
+    // sum up the room of frames below of the current one
-    for (fr = curfr->fr_next; fr != NULL; fr = fr->fr_next)
+    FOR_ALL_FRAMES(fr, curfr->fr_next) {
       room += fr->fr_height - frame_minheight(fr, NULL);
-    fr = curfr;                         /* put fr at window that grows */
+    }
+    fr = curfr;  // put fr at window that grows
   }
 
   if (room < offset)            /* Not enough room */
@@ -5399,9 +5414,10 @@ void win_drag_vsep_line(win_T *dragwin, int offset)
     left = FALSE;
     /* sum up the room of frames right of the current one */
     room = 0;
-    for (fr = curfr->fr_next; fr != NULL; fr = fr->fr_next)
+    FOR_ALL_FRAMES(fr, curfr->fr_next) {
       room += fr->fr_width - frame_minwidth(fr, NULL);
-    fr = curfr;                         /* put fr at window that grows */
+    }
+    fr = curfr;  // put fr at window that grows
   }
   assert(fr);
 
@@ -5933,9 +5949,10 @@ static void last_status_rec(frame_T *fr, int statusline)
       redraw_all_later(SOME_VALID);
     }
   } else if (fr->fr_layout == FR_ROW) {
-    /* vertically split windows, set status line for each one */
+    // vertically split windows, set status line for each one
-    for (fp = fr->fr_child; fp != NULL; fp = fp->fr_next)
+    FOR_ALL_FRAMES(fp, fr->fr_child) {
       last_status_rec(fp, statusline);
+    }
   } else {
     /* horizontally split window, set status line for last one */
     for (fp = fr->fr_child; fp->fr_next != NULL; fp = fp->fr_next)
@@ -6551,14 +6568,15 @@ matchitem_T *get_match(win_T *wp, int id)
 ///
 /// @param  topfrp  top frame pointer
 /// @param  height  expected height
-static bool frame_check_height(frame_T *topfrp, int height)
+static bool frame_check_height(const frame_T *topfrp, int height)
   FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL
 {
   if (topfrp->fr_height != height) {
     return false;
   }
   if (topfrp->fr_layout == FR_ROW) {
-    for (frame_T *frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next) {
+    const frame_T *frp;
+    FOR_ALL_FRAMES(frp, topfrp->fr_child) {
       if (frp->fr_height != height) {
         return false;
       }
@@ -6571,14 +6589,15 @@ static bool frame_check_height(frame_T *topfrp, int height)
 ///
 /// @param  topfrp  top frame pointer
 /// @param  width   expected width
-static bool frame_check_width(frame_T *topfrp, int width)
+static bool frame_check_width(const frame_T *topfrp, int width)
   FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL
 {
   if (topfrp->fr_width != width) {
     return false;
   }
   if (topfrp->fr_layout == FR_COL) {
-    for (frame_T *frp = topfrp->fr_child; frp != NULL; frp = frp->fr_next) {
+    const frame_T *frp;
+    FOR_ALL_FRAMES(frp, topfrp->fr_child) {
       if (frp->fr_width != width) {
         return false;
       }