Subject: Re: bug#49160: 28.0.50; [PATCH] Uninitialized inhibit_buffer_hooks

From: "Basil L. Contovounesios" <[email-masked]>
Date: Wed, 11 Jan 2023 20:13:21 +0000

Lars Ingebrigtsen [2021-07-19 18:23 +0200] wrote:

> Eli Zaretskii <[email-masked]> writes:
>
>> More to the point: since this attribute of a buffer cannot be changed
>> once the buffer is created, I think it might be okay to copy from the
>> base buffer, but we should also allow the caller to control that by
>> introducing an optional argument similar to that of get-buffer-create.
>> And if we do that, we could also consider making the default value be
>> nil, not necessarily a copy of that of the base buffer.
>
> I've now applied Dick's patch, and then adjusted as you suggest.

Fmake_indirect_buffer continues to run buffer-list-update-hook
regardless of the new argument.  Any objections to changing that?

>From 0fdf0cb925088d0b5a019df2bbf803e550bebbd5 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <[email-masked]>
Date: Wed, 11 Jan 2023 16:34:07 +0000
Subject: [PATCH] Fix buffer-list-update-hook for indirect buffers

Fmake_indirect_buffer can be told whether to run buffer hooks since
bug#49160, but until now it ran buffer-list-update-hook irrespective
of this.

* src/buffer.c (Fmake_indirect_buffer): Don't run
buffer-list-update-hook when called with a non-nil
INHIBIT-BUFFER-HOOKS argument.
(run_buffer_list_update_hook): Don't special-case NULL argument, as
no such callers remain.

* test/src/buffer-tests.el
(buffer-tests-inhibit-buffer-hooks-indirect): Test whether indirect
buffer hooks are run regardless of whether base buffer hooks are
inhibited.  Check that all three buffer hooks, not just
kill-buffer-query-functions, are inhibited.
---
 src/buffer.c             | 10 ++++-----
 test/src/buffer-tests.el | 46 +++++++++++++++++++++++-----------------
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index 100e42fc1f9..88ca69b0dd8 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -525,14 +525,14 @@ get_truename_buffer (register Lisp_Object filename)
   return Qnil;
 }
 
-/* Run buffer-list-update-hook if Vrun_hooks is non-nil, and BUF is NULL
-   or does not have buffer hooks inhibited.  BUF is NULL when called by
-   make-indirect-buffer, since it does not inhibit buffer hooks.  */
+/* Run buffer-list-update-hook if Vrun_hooks is non-nil and BUF does
+   not have buffer hooks inhibited.  */
 
 static void
 run_buffer_list_update_hook (struct buffer *buf)
 {
-  if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks)))
+  eassert (buf);
+  if (! (NILP (Vrun_hooks) || buf->inhibit_buffer_hooks))
     call1 (Vrun_hooks, Qbuffer_list_update_hook);
 }
 
@@ -907,7 +907,7 @@ DEFUN ("make-indirect-buffer", Fmake_indirect_buffer, Smake_indirect_buffer,
       set_buffer_internal_1 (old_b);
     }
 
-  run_buffer_list_update_hook (NULL);
+  run_buffer_list_update_hook (b);
 
   return buf;
 }
diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el
index e5de8f3464a..9d4bbf3e040 100644
--- a/test/src/buffer-tests.el
+++ b/test/src/buffer-tests.el
@@ -8315,29 +8315,35 @@ buffer-tests-inhibit-buffer-hooks
       (remove-hook 'buffer-list-update-hook bluh))))
 
 (ert-deftest buffer-tests-inhibit-buffer-hooks-indirect ()
-  "Indirect buffers do not call `get-buffer-create'."
-  (dolist (inhibit '(nil t))
-    (let ((base (get-buffer-create "foo" inhibit)))
+  "Test `make-indirect-buffer' argument INHIBIT-BUFFER-HOOKS."
+  (let* ( base run-bluh run-kbh run-kbqf
+          (bluh (lambda () (setq run-bluh t)))
+          (kbh  (lambda () (setq run-kbh  t)))
+          (kbqf (lambda () (setq run-kbqf t))))
+    (dolist (inhibit-base '(nil t))
       (unwind-protect
-          (dotimes (_i 11)
-            (let* (flag*
-                   (flag (lambda () (prog1 t (setq flag* t))))
-                   (indirect (make-indirect-buffer base "foo[indirect]" nil
-                                                   inhibit)))
-              (unwind-protect
-                  (progn
-                    (with-current-buffer indirect
-                      (add-hook 'kill-buffer-query-functions flag nil t))
-                    (kill-buffer indirect)
-                    (if inhibit
-                        (should-not flag*)
-                      (should flag*)))
-                (let (kill-buffer-query-functions)
+          (let (indirect)
+            (setq base (generate-new-buffer " base" inhibit-base))
+            (dolist (inhibit-indirect '(nil t))
+              (dotimes (_ 11)
+                (unwind-protect
+                    (let ((name (generate-new-buffer-name " indirect")))
+                      (setq run-bluh nil run-kbh nil run-kbqf nil)
+                      (add-hook 'buffer-list-update-hook bluh)
+                      (with-current-buffer
+                          (setq indirect (make-indirect-buffer
+                                          base name nil inhibit-indirect))
+                        (add-hook 'kill-buffer-hook kbh nil t)
+                        (add-hook 'kill-buffer-query-functions kbqf nil t)
+                        (kill-buffer))
+                      (should (xor inhibit-indirect run-bluh))
+                      (should (xor inhibit-indirect run-kbh))
+                      (should (xor inhibit-indirect run-kbqf)))
+                  (remove-hook 'buffer-list-update-hook bluh)
                   (when (buffer-live-p indirect)
                     (kill-buffer indirect))))))
-        (let (kill-buffer-query-functions)
-          (when (buffer-live-p base)
-            (kill-buffer base)))))))
+        (when (buffer-live-p base)
+          (kill-buffer base))))))
 
 (ert-deftest zero-length-overlays-and-not ()
   (with-temp-buffer
-- 
2.39.0

Thanks,

-- 
Basil