Subject: bug#43192: lisp/files.el; 6d10b607d0 introduced bug that breaks C-x C-c

From: Tom Gillespie <[email-masked]>
Date: Thu, 3 Sep 2020 19:49:33 -0700

Trying to run save-buffers-kill-terminal from a buffer that
has no buffer-file-name fails with stringp, nil if there is
more than one buffer with the same file name (which appends
<> to the buffer name) and one of those buffers is unsaved.

Here is the repro.
#+begin_src bash
emacs -Q --eval "\
(progn
  (setq auto-save-default nil)
  (ignore-errors (make-directory \"/tmp/a\"))
  (ignore-errors (make-directory \"/tmp/b\"))
  (with-current-buffer (find-file \"/tmp/a/file.ext\")
    (erase-buffer)
    (insert \"\n\")
    (save-buffer))
  (with-current-buffer (find-file \"/tmp/b/file.ext\")
    (erase-buffer)
    (save-buffer)
    (insert \"some text\n\"))
  (switch-to-buffer \"*scratch*\")
  (with-current-buffer \"*scratch*\"
    (save-buffers-kill-terminal)))"
#+end_src

If you add a second (save-buffer) after (insert \"some text\n\")
then emacs will exit as expected. The repro can also be run
in batch mode and will exit with code 255 instead of 0, and
could be added as a test to prevent this in the future.

This happens because the *scratch* buffer does not have a
buffer-file-name, and that branch of the or statement is reached
because I have two files with the same name in different folders open
and thus the buffer file name does not match the buffer name so it
goes and looks at scratch which has no buffer file name at all,
causing the stringp, nil error.

The offending lines from 6d10b607d094bfd29b9ce0c4baf469e3683c3ac6
#+begin_src diff
+                                   (string-match
+                                    (concat "\\<"
+                                            (regexp-quote
+                                             (file-name-nondirectory
+                                              buffer-file-name))
+                                            "<[0-9]+>\\'")
+                                    (buffer-name buffer)))
#+end_src

This is the second statement in a call to `or'. buffer-file-name is
not guaranteed to be non-nil because buffers like *scratch* and
*Messages* exist. In many workflows for emacsclient opening to scratch
and closing again from scratch are common.

I think that putting the string-match inside (if buffer-file-name ...)
should be sufficient to fix the issue, but I don't know what the other
branch should be, if anything. I worry that there may also be other
issues with incorrect assumptions about the relationship between
buffer-name and buffer-file-name when there is more than one file
with the same name open, but I have not checked carefully.