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

Sean Whitton <[email protected]> writes:

> Donjuanplatinum [30/Jul  9:35pm +08] wrote:
>> 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.
>
> I see what you mean.
>
> I can't install the full patch because I don't think you've assigned
> copyright for contributions to Emacs to the FSF.  If you'd be interested
> in doing that we can start that process?
>
> In the meantime I've installed the C changes, because it does not seem
> essential to have a test for this.  But after copyright assignment we
> can install the test anyway.
Thank you. Yes, i'd happy to start the FSF Copyright assignment
process.Please let me know the next steps.