bug#54380: 29.0.50; Unexpected nil value of 'rest' in 'balance-windows-2'

bug#54380: 29.0.50; Unexpected nil value of 'rest' in 'balance-windows-2'

[Matt Beshara]

Attempting to use ‘balance-windows’ in a frame displaying many 
windows, two of which are displaying buffers which both have the 
buffer-local variable ‘window-size-fixed’ set to t, fails with the 
error “Wrong type argument: number-or-marker-p, nil”.  This error 
is produced by line 5765 of window.el, “(while (and sub (> rest 
0))”.  It appears that the code in this function always expects 
‘rest’ to have a non-nil value, but in this case ‘rest’ is nil. 
Modifying line 5765 to read “(while (and sub rest (> rest 0))” so 
that it checks ‘rest’ is non-nil before attemping to call ‘>’ 
seems to fix it.  I haven’t attached a patch as a patch for a 
one-line diff seems excessive, but if it would be useful, I can 
provide one.

Steps to replicate:
 1. emacs -Q
 2. C-x 2
 3. C-x o
 4. C-u 10 M-x shrink-window
 5. M-: (setq-local window-size-fixed t)
 6. C-x o
 7. C-x b *Messages*
 8. C-x 3
 9. C-x o
10. C-x C-f /tmp/test
11. C-u 10 C-x {
12. M-: (setq-local window-size-fixed t)
13. C-x o
14. C-x o
15. C-x 2
16. C-x 2
17. C-x 2
18. C-x 3
19. C-x 3
20. M-x balance-windows-area ;; This works as one would expect
21. M-x balance-windows      ;; This produces an error

bug#54380: 29.0.50; Unexpected nil value of 'rest' in 'balance-windows-2'

[martin rudalics]

If the code had used a term like say 'number-of-resizable-children' we
might have been able to avoid this bug in the first place.  Too bad ...

 > Attempting to use ‘balance-windows’ in a frame displaying many
 > windows, two of which are displaying buffers which both have the
 > buffer-local variable ‘window-size-fixed’ set to t, fails with the
 > error “Wrong type argument: number-or-marker-p, nil”.  This error is
 > produced by line 5765 of window.el, “(while (and sub (> rest 0))”.  It
 > appears that the code in this function always expects ‘rest’ to have a
 > non-nil value, but in this case ‘rest’ is nil. Modifying line 5765 to
 > read “(while (and sub rest (> rest 0))” so that it checks ‘rest’ is
 > non-nil before attemping to call ‘>’ seems to fix it.  I haven’t
 > attached a patch as a patch for a one-line diff seems excessive, but
 > if it would be useful, I can provide one.

Thanks for the report.  The bug is a consequence of the following
sequence of steps in 'balance-windows-2':

- If an internal window has only fixed-size children, we eventually set
   'number-of-children' to zero here

	    (setq number-of-children (1- number-of-children))

- We set 'rest' only if 'number-of-children' is greater zero here

     (when (> number-of-children 0)
       (setq rest (% total-sum number-of-children))

- We ask for the value of 'rest' when a window has at least one child as

   (let* (...
          (first (window-child window))

     (setq sub first)
     (while (and sub (> rest 0))

I checked in a fix, please try with latest master.

Thanks again, martin

bug#54380: 29.0.50; Unexpected nil value of 'rest' in 'balance-windows-2'

[Matt Beshara]

On 14 March 2022 8:30:40 pm AEDT, martin rudalics <rudalics@gmx.at> wrote:
>If the code had used a term like say 'number-of-resizable-children' we
>might have been able to avoid this bug in the first place.  Too bad ...
>
> > Attempting to use ‘balance-windows’ in a frame displaying many
> > windows, two of which are displaying buffers which both have the
> > buffer-local variable ‘window-size-fixed’ set to t, fails with the
> > error “Wrong type argument: number-or-marker-p, nil”.  This error is
> > produced by line 5765 of window.el, “(while (and sub (> rest 0))”.  It
> > appears that the code in this function always expects ‘rest’ to have a
> > non-nil value, but in this case ‘rest’ is nil. Modifying line 5765 to
> > read “(while (and sub rest (> rest 0))” so that it checks ‘rest’ is
> > non-nil before attemping to call ‘>’ seems to fix it.  I haven’t
> > attached a patch as a patch for a one-line diff seems excessive, but
> > if it would be useful, I can provide one.
>
>Thanks for the report.  The bug is a consequence of the following
>sequence of steps in 'balance-windows-2':
>
>- If an internal window has only fixed-size children, we eventually set
>   'number-of-children' to zero here
>
>	    (setq number-of-children (1- number-of-children))
>
>- We set 'rest' only if 'number-of-children' is greater zero here
>
>     (when (> number-of-children 0)
>       (setq rest (% total-sum number-of-children))
>
>- We ask for the value of 'rest' when a window has at least one child as
>
>   (let* (...
>          (first (window-child window))
>
>     (setq sub first)
>     (while (and sub (> rest 0))
>
>I checked in a fix, please try with latest master.
>
>Thanks again, martin

The fix works very well, thank you.

bug#54380: 29.0.50; Unexpected nil value of 'rest' in 'balance-windows-2'

[martin rudalics]

close 54380 29.1
quit

 > The fix works very well, thank you.

Thanks for checking.  Closing this bug.

martin