bug#81504: [PATCH] Restore current buffer after clone-indirect-buffer-hook errors

Sean Whitton <[email protected]> writes:

> Donjuanplatinum via "Bug reports for GNU Emacs, the Swiss army knife of text editors" [28/Jul  1:47am +08] wrote:
>> Version: 31.0.50
>> Base-Commit: 810e9d675ba484dabade1a9f30201d2848c5e2ea
>>
>> In `make-indirect-buffer` with clone argument, if
>> `clone-indirect-buffer-hook` return an error, `current-buffer` will not
>> be restored.
>>
>> To make the error in emacs -Q:
>>
>>
>>    (progn
>>       (generate-new-buffer "base")
>>       (with-current-buffer "base" (insert "text"))
>>       (set-buffer "base")
>>       (let ((clone-indirect-buffer-hook
>>              (list (lambda () (error "hook error")))))
>>         (condition-case nil
>>             (make-indirect-buffer "base" "ind" t)
>>           (error nil)))
>>       (buffer-name (current-buffer)))
>>
>> Expected: "base"
>> But get: "ind"
>>
>> The Clone path in Fmake_indirect_buffer use set_buffer_internal_1 to
>> restore the current-buffer, but when clone-indirect-buffer-hook return
>> an error, it will skip the set_buffer_internal_1.
>>
>> The attached patch replaces it with record_unwind_current_buffer so that
>> it will following the standard specpdl pattern to restore the current-buffer.
>
> Should we be restoring the current buffer here?  I'm not sure there is
> any particular reason why we should, and calling code can always use
> with-current-buffer or save-current-buffer if it needs to ensure that
> it's restored?
Thank you for taking a look.

I think the function already intends to restore the current buffer on
the normal path. Otherwise, why does it save old_b and call
set_buffer_internal_1 (old_b) before returning?

My patch only makes the error path behave same with the normal path.