GNU bug report logs - #43192
lisp/files.el; 6d10b607d0 introduced bug that breaks C-x C-c

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43192 in the body.
You can then email your comments to 43192 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tom Gillespie <tgbugs <at> gmail.com>
To: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>
Subject: lisp/files.el; 6d10b607d0 introduced bug that breaks C-x C-c
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.

Message #8 received at 43192 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Tom Gillespie <tgbugs <at> gmail.com>
Cc: 43192 <at> debbugs.gnu.org
Subject: Re: bug#43192: lisp/files.el; 6d10b607d0 introduced bug that breaks
 C-x C-c
Date: Fri, 04 Sep 2020 04:58:54 +0200

Tom Gillespie <tgbugs <at> gmail.com> writes:

> #+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 it's just a typo -- the code should be:

diff --git a/lisp/files.el b/lisp/files.el
index 3403e257a1..5f5902d0cb 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5574,7 +5574,7 @@ save-some-buffers
                                     (concat "\\<"
                                             (regexp-quote
                                              (file-name-nondirectory
-                                              buffer-file-name))
+                                              (buffer-file-name buffer)))
                                             "<[^>]*>\\'")
                                     (buffer-name buffer)))
                                   ;; The buffer name is similar to the

I've now applied this to Emacs 28, and that fixes the test case in this
bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #15 received at 43192 <at> debbugs.gnu.org (full text, mbox):

From: Tom Gillespie <tgbugs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 43192 <at> debbugs.gnu.org
Subject: Re: bug#43192: lisp/files.el; 6d10b607d0 introduced bug that breaks
 C-x C-c
Date: Thu, 3 Sep 2020 20:16:09 -0700

Ah, I wondered if that might be the case, everything else was so
consistent, but I thought there might be some reason to use the
variable directly. Since it is not the case my other concerns don't
matter. Confirming fixed on my end. Thanks!
Tom

On Thu, Sep 3, 2020 at 7:59 PM Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
>
> Tom Gillespie <tgbugs <at> gmail.com> writes:
>
> > #+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 it's just a typo -- the code should be:
>
> diff --git a/lisp/files.el b/lisp/files.el
> index 3403e257a1..5f5902d0cb 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -5574,7 +5574,7 @@ save-some-buffers
>                                      (concat "\\<"
>                                              (regexp-quote
>                                               (file-name-nondirectory
> -                                              buffer-file-name))
> +                                              (buffer-file-name buffer)))
>                                              "<[^>]*>\\'")
>                                      (buffer-name buffer)))
>                                    ;; The buffer name is similar to the
>
> I've now applied this to Emacs 28, and that fixes the test case in this
> bug report.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.