bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Daniel Sutton]

[-- Attachment #1: Type: text/plain, Size: 7521 bytes --]

There seems to be a bug with `file-name-case-insensitive-p` when given a
file name that doesn't exist and does not have a root that exists. For
instance `(file-name-case-insensitive-p "some/project")`.

Unfortunately I'm unable to give a simple reproduction as this only
seemed to happen when running tests for CIDER under cask. This behavior
seems to have been introduced in
81d6418e6b7c3e637dccf9c856d9c4b94bd43b97 with the commit message:

Fix file-name-case-insensitive-p on non-existent files

* src/fileio.c (Ffile_name_case_insensitive_p): If the file
doesn't exist, move up the filesystem tree until an existing
directory is found.  Then test that directory for
case-insensitivity.  (Bug#32246)

https://github.com/clojure-emacs/cider/pull/2669/commits/2039abd3c48214200a9e50a5288246011d48eb3e

Configured features:
XPM JPEG TIFF GIF PNG SOUND DBUS GSETTINGS GLIB NOTIFY INOTIFY
LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11 XDBE XIM THREADS LIBSYSTEMD PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Dired by name

Minor modes in effect:
  recentf-mode: t
  global-magit-file-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  async-bytecomp-package-mode: t
  shell-dirtrack-mode: t
  ivy-mode: t
  projectile-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  global-undo-tree-mode: t
  delete-selection-mode: t
  company-quickhelp-mode: t
  company-quickhelp-local-mode: t
  global-company-mode: t
  company-mode: t
  which-key-mode: t
  global-hl-line-mode: t
  minions-mode: t
  pixel-scroll-mode: t
  save-place-mode: t
  winner-mode: t
  show-paren-mode: t
  global-auto-revert-mode: t
  auto-compile-on-load-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
~/projects/dev/clojure-mode/clojure-mode hides
/home/dan/.emacs.d/elpa/clojure-mode-20190508.1522/clojure-mode

Features:
(shadow sort mail-extr emacsbug sendmail whitespace tabify magit-patch
loccur cl-print eieio-opt speedbar sb-image ezimage dframe recentf
bug-reference cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs help-fns radix-tree cider tramp-sh
cider-debug cider-inspector cider-browse-ns cider-mode cider-completion
cider-profile cider-eval cider-repl-history pulse cider-repl
cider-resolve cider-test cider-overlays cider-stacktrace cider-doc
cider-browse-spec org-table org-element avl-tree org org-macro
org-footnote org-pcomplete org-list org-faces org-entities org-version
ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table ob-keys ob-exp
ob-comint ob-core ob-eval org-compat org-macs org-loaddefs cal-menu
calendar cal-loaddefs cider-grimoire cider-popup cider-eldoc
cider-client cider-common cider-util cider-connection sesman-browser
nrepl-client nrepl-dict cider-compat crux tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat parse-time ls-lisp vc-git
checkdoc magit-extras magit-bookmark magit-submodule magit-obsolete
magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull
magit-fetch magit-clone magit-remote magit-commit magit-sequence
magit-notes magit-worktree magit-tag magit-merge magit-branch
magit-reset magit-files magit-refs magit-status magit magit-repos
magit-apply magit-wip magit-log which-func magit-diff smerge-mode
diff-mode magit-core magit-autorevert magit-margin magit-transient
magit-process magit-mode transient git-commit magit-git magit-section
magit-utils crm log-edit message rfc822 mml mml-sec epa epg gnus-util
rmail rmail-loaddefs text-property-search time-date mm-decode mm-bodies
mm-encode mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log
with-editor async-bytecomp async shell pcomplete counsel xdg dired
dired-loaddefs swiper ivy colir ivy-overlay ffap smart-mode-line
rich-minority projectile grep ibuf-ext ibuffer ibuffer-loaddefs
company-oddmuse company-keywords company-etags etags fileloop generator
company-gtags company-dabbrev-code company-dabbrev company-files
company-capf company-cmake company-xcode company-clang company-semantic
company-eclim company-template company-bbdb lsp-clojure lsp-mode
markdown-mode noutline outline tree-widget xref network-stream inline ht
f s em-glob esh-util dash-functional flymake-proc flymake thingatpt
clojure-mode project align imenu buttercup warnings ert ewoc debug
backtrace buttercup-compat flycheck-joker sesman vc vc-dispatcher
spinner queue pkg-info url-http url url-proxy url-privacy url-expand
url-methods url-history mailcap url-auth mail-parse rfc2231 rfc2047
rfc2045 mm-util ietf-drums mail-prsvr url-cookie url-domsuf url-util
url-gw nsm rmc puny lisp-mnt epl parseedn parseclj-parser parseclj-lex a
yasnippet elec-pair flycheck find-func rainbow-delimiters paredit
pcre2el rxt re-builder rx pdf-tools compile comint ansi-color pdf-view
bookmark pp pdf-cache pdf-info tq pdf-util format-spec image-mode
undo-tree diff delsel browse-kill-ring derived cl all-the-icons
all-the-icons-faces data-material data-weathericons data-octicons
data-fileicons data-faicons data-alltheicons memoize company-quickhelp
pos-tip company pcase hydra lv diminish which-key gruvbox-theme gruvbox
sublime-themes kaolin-themes kaolin-themes-lib darktooth-theme
autothemer solarized-theme solarized color hl-line moody minions dash
pixel-scroll advice saveplace winner ring paren autorevert filenotify
edmacro kmacro resize-window cl-extra help-mode jka-compr server
auto-compile packed use-package use-package-ensure use-package-delight
use-package-diminish use-package-bind-key bind-key easy-mmode
use-package-core finder-inf cus-edit cus-start cus-load wid-edit
mule-util info package easymenu epg-config url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 641932 908328)
 (symbols 48 45885 1184)
 (strings 32 196006 120504)
 (string-bytes 1 5710715)
 (vectors 16 81080)
 (vector-slots 8 1615861 884472)
 (floats 8 972 10098)
 (intervals 56 13745 17676)
 (buffers 992 85))

[-- Attachment #2: Type: text/html, Size: 8225 bytes --]

bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/4/2019 12:48 PM, Daniel Sutton wrote:
> There seems to be a bug with `file-name-case-insensitive-p` when given a
> file name that doesn't exist and does not have a root that exists. For
> instance `(file-name-case-insensitive-p "some/project")`.

The definition of file-name-case-insensitive-p begins by converting the argument 
to an absolute file name via the C equivalent of

   (expand-file-name filename nil)

This is supposed to produce a file name that has a root that exists.

> Unfortunately I'm unable to give a simple reproduction as this only
> seemed to happen when running tests for CIDER under cask.

Can you investigate what goes wrong with expand-file-name in your setting?

Ken

bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

Please keep the bug address in the Cc.

On 7/4/2019 3:35 PM, Daniel Sutton wrote:
> Locally when running `(expand-file-name "path/to/dirA/" nil)` it expands 
> to "/home/dan/projects/dev/cider/test/path/to/dirA/"
> 
> when running under cask it expands to "/path/to/dirA/"

So moving up the file system tree should eventually lead you to "/".  I don't 
see why you're getting an infinite loop.

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Daniel Sutton]

[-- Attachment #1: Type: text/plain, Size: 3570 bytes --]

(repeated email as I omitted the bug email address)

Ken I believe I've figured it out and there was an important bit I left out.

The test in CIDER was ensuring a buffer was named correctly based on the
current project, and it set the current project's directory by setq'ing
default-directory. This seems to be a big no-no as the default directory
expects a well formed path. The doc string of default-directory being:

Name of default directory of current buffer.
It should be an absolute directory name; on GNU and Unix systems,
these names start with `/' or `~' and end with `/'.
To interactively change the default directory, use command `cd'.

So the "bug" can be reproduced by the following, which was the important
bits of the test:

(let ((default-directory "made/up/project/location"))
  (file-name-case-insensitive-p default-directory))

* note this will freeze your emacs

I believe your change caused the behavior here but it seems like a
reasonable change and CIDER is definitely not honoring the assumptions in
`default-directory`. We had some careless usages of this variable in our
tests and it seems like it has finally caught up to us. Unless you consider
this an edge case to watch for (and I can't imagine you do) it seems like
this should be closed.

Sorry for the noise and thanks for your help
Dan Sutton

On Thu, Jul 4, 2019 at 9:36 PM Daniel Sutton <dan@dpsutton.com> wrote:

> First thanks so much for your quick attention, Ken. I really appreciate
> it. I threw in some printlning and seeing the following output:
>
> originally: path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/
>
> filename from c:
> path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA
>
> filename from c:
> path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA
>
> filename from c:
> path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA
>
> filename from c:
> path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA
>
> filename from c:
> path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA/path/to/dirA
>
> from the following:
>
> if (NILP (Ffile_exists_p (filename)))
>   {
>     filename = Ffile_name_directory (filename);
>     while (NILP (Ffile_exists_p (filename)))
>       {
>         Lisp_Object newname = expand_and_dir_to_file (filename);
>         /* Avoid infinite loop if the root is reported as non-existing
>            (impossible?).  */
>         if (!NILP (Fstring_equal (newname, filename)))
>           break;
>         filename = newname;
>         message_with_string("filename from c: %s\n", filename, 1);
>       }
>   }
>
> So it seems to be growing the wrong way. I suppose this is the problem
> actually in expand_and_dir_to_file but i can't imagine why.
>
> On Thu, Jul 4, 2019 at 8:32 PM Ken Brown <kbrown@cornell.edu> wrote:
>
>> Please keep the bug address in the Cc.
>>
>> On 7/4/2019 3:35 PM, Daniel Sutton wrote:
>> > Locally when running `(expand-file-name "path/to/dirA/" nil)` it
>> expands
>> > to "/home/dan/projects/dev/cider/test/path/to/dirA/"
>> >
>> > when running under cask it expands to "/path/to/dirA/"
>>
>> So moving up the file system tree should eventually lead you to "/".  I
>> don't
>> see why you're getting an infinite loop.
>>
>> Ken
>>
>

[-- Attachment #2: Type: text/html, Size: 4751 bytes --]

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/4/2019 11:05 PM, Daniel Sutton wrote:
> Sorry for the noise and thanks for your help

No problem.  I'm glad you solved it.

Closing.

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Noam Postavsky]

Daniel Sutton <dan@dpsutton.com> writes:

> So the "bug" can be reproduced by the following, which was the important
> bits of the test:
>
> (let ((default-directory "made/up/project/location"))
>   (file-name-case-insensitive-p default-directory))
>
> * note this will freeze your emacs
>
> I believe your change caused the behavior here but it seems like a
> reasonable change and CIDER is definitely not honoring the assumptions in
> `default-directory`.

>> From: Ken Brown <kbrown@cornell.edu>
>
>> On 7/4/2019 11:05 PM, Daniel Sutton wrote:
>> > Sorry for the noise and thanks for your help
>
>> No problem.  I'm glad you solved it.
>
>> Closing.

Wouldn't it be better not to infloop in this case though?  E.g.,

--- i/src/fileio.c
+++ w/src/fileio.c
@@ -2408,7 +2408,10 @@ DEFUN ("file-name-case-insensitive-p", Ffile_name_case_insensitive_p,
 
   /* If the file doesn't exist, move up the filesystem tree until we
      reach an existing directory or the root.  */
-  if (NILP (Ffile_exists_p (filename)))
+  if (NILP (Ffile_exists_p (filename))
+      /* If default-directory is relative, expand-file-name can give
+         a relative name, in which case we can't move up.  */
+      && !NILP (Ffile_name_absolute_p (filename)))
     {
       filename = Ffile_name_directory (filename);
       while (NILP (Ffile_exists_p (filename)))

Or maybe signal an error, either way seems better than just getting stuck.

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/6/2019 9:27 AM, Noam Postavsky wrote:
> Wouldn't it be better not to infloop in this case though?  E.g.,
> 
> --- i/src/fileio.c
> +++ w/src/fileio.c
> @@ -2408,7 +2408,10 @@ DEFUN ("file-name-case-insensitive-p", Ffile_name_case_insensitive_p,
>   
>     /* If the file doesn't exist, move up the filesystem tree until we
>        reach an existing directory or the root.  */
> -  if (NILP (Ffile_exists_p (filename)))
> +  if (NILP (Ffile_exists_p (filename))
> +      /* If default-directory is relative, expand-file-name can give
> +         a relative name, in which case we can't move up.  */

This doesn't seem right to me.  expand-file-name is documented to return 
an absolute file name, so I don't think callers should have to check for 
that.

> +      && !NILP (Ffile_name_absolute_p (filename)))
>       {
>         filename = Ffile_name_directory (filename);
>         while (NILP (Ffile_exists_p (filename)))
> 
> Or maybe signal an error, either way seems better than just getting stuck.

Maybe expand-file-name should signal an error if default-directory is 
relative?

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> Date: Sat, 6 Jul 2019 15:38:14 +0000
> Cc: "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>
> 
> > +      && !NILP (Ffile_name_absolute_p (filename)))
> >       {
> >         filename = Ffile_name_directory (filename);
> >         while (NILP (Ffile_exists_p (filename)))
> > 
> > Or maybe signal an error, either way seems better than just getting stuck.
> 
> Maybe expand-file-name should signal an error if default-directory is 
> relative?

That'd not be my first preference.  My first preference would be to
detect the cases where we are about to loop.  Can we do that?

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/6/2019 12:14 PM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown@cornell.edu>
>> Date: Sat, 6 Jul 2019 15:38:14 +0000
>> Cc: "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>
>>
>>> +      && !NILP (Ffile_name_absolute_p (filename)))
>>>        {
>>>          filename = Ffile_name_directory (filename);
>>>          while (NILP (Ffile_exists_p (filename)))
>>>
>>> Or maybe signal an error, either way seems better than just getting stuck.
>>
>> Maybe expand-file-name should signal an error if default-directory is
>> relative?
> 
> That'd not be my first preference.  My first preference would be to
> detect the cases where we are about to loop.  Can we do that?

The only case I can think of where we might loop is the case we're discussing, 
in which expand-file-name fails to return an absolute name.  Noam's patch is 
fine for that case.  But wouldn't it be better to fix expand-file-name so that 
it always returns an absolute name, as it's documented to do?

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> CC: "npostavs@gmail.com" <npostavs@gmail.com>, "dan@dpsutton.com"
> 	<dan@dpsutton.com>, "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>
> Date: Sun, 7 Jul 2019 14:09:03 +0000
> 
> wouldn't it be better to fix expand-file-name so that it always
> returns an absolute name, as it's documented to do?

I think it's better, yes.  Do we have an idea for how to do that?

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/7/2019 10:37 AM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown@cornell.edu>
>> CC: "npostavs@gmail.com" <npostavs@gmail.com>, "dan@dpsutton.com"
>> 	<dan@dpsutton.com>, "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>
>> Date: Sun, 7 Jul 2019 14:09:03 +0000
>>
>> wouldn't it be better to fix expand-file-name so that it always
>> returns an absolute name, as it's documented to do?
> 
> I think it's better, yes.  Do we have an idea for how to do that?

How's this?

--- a/src/fileio.c
+++ b/src/fileio.c
@@ -804,7 +804,11 @@ DEFUN ("expand-file-name", Fexpand_file_name, 
Sexpand_file_name, 1, 2, 0,

    /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
    if (NILP (default_directory))
-    default_directory = BVAR (current_buffer, directory);
+    {
+      default_directory = BVAR (current_buffer, directory);
+      if (NILP (Ffile_name_absolute_p (default_directory)))
+       default_directory = Qnil;
+    }
    if (! STRINGP (default_directory))
      {
  #ifdef DOS_NT

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> CC: "npostavs@gmail.com" <npostavs@gmail.com>, "dan@dpsutton.com"
> 	<dan@dpsutton.com>, "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>
> Date: Sun, 7 Jul 2019 19:30:00 +0000
> 
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -804,7 +804,11 @@ DEFUN ("expand-file-name", Fexpand_file_name, 
> Sexpand_file_name, 1, 2, 0,
> 
>     /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
>     if (NILP (default_directory))
> -    default_directory = BVAR (current_buffer, directory);
> +    {
> +      default_directory = BVAR (current_buffer, directory);
> +      if (NILP (Ffile_name_absolute_p (default_directory)))
> +       default_directory = Qnil;
> +    }

Hmm... why nullify it?  Why not simply call expand-file-name
recursively?

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/8/2019 8:25 AM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown@cornell.edu>
>> CC: "npostavs@gmail.com" <npostavs@gmail.com>, "dan@dpsutton.com"
>> 	<dan@dpsutton.com>, "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>
>> Date: Sun, 7 Jul 2019 19:30:00 +0000
>>
>> --- a/src/fileio.c
>> +++ b/src/fileio.c
>> @@ -804,7 +804,11 @@ DEFUN ("expand-file-name", Fexpand_file_name,
>> Sexpand_file_name, 1, 2, 0,
>>
>>      /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
>>      if (NILP (default_directory))
>> -    default_directory = BVAR (current_buffer, directory);
>> +    {
>> +      default_directory = BVAR (current_buffer, directory);
>> +      if (NILP (Ffile_name_absolute_p (default_directory)))
>> +       default_directory = Qnil;
>> +    }
> 
> Hmm... why nullify it?  Why not simply call expand-file-name
> recursively?

If the current buffer's default-directory is not absolute, then that variable is 
invalid and we can't use it.  (This was perhaps not clear until last November, 
when the word "absolute" was added to its doc string.)

Nullifying it guarantees that the code starting with "if (! STRINGP 
(default_directory))" is used.  Maybe I should have put in a comment explaining 
that.

My thinking was that an invalid value of the default-directory variable should 
be treated the same way we treat a non-string value of default_directory.

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> CC: "npostavs@gmail.com" <npostavs@gmail.com>, "dan@dpsutton.com"
> 	<dan@dpsutton.com>, "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>
> Date: Mon, 8 Jul 2019 13:36:38 +0000
> 
> >>      if (NILP (default_directory))
> >> -    default_directory = BVAR (current_buffer, directory);
> >> +    {
> >> +      default_directory = BVAR (current_buffer, directory);
> >> +      if (NILP (Ffile_name_absolute_p (default_directory)))
> >> +       default_directory = Qnil;
> >> +    }
> > 
> > Hmm... why nullify it?  Why not simply call expand-file-name
> > recursively?
> 
> If the current buffer's default-directory is not absolute, then that variable is 
> invalid and we can't use it.

I'm asking why not do this instead:

  if (NILP (default_directory))
    {
      default_directory = BVAR (current_buffer, directory);
      if (NILP (Ffile_name_absolute_p (default_directory)))
        default_directory = Fexpand_file_name (default_directory,
	                                       Vinvocation_directory);
    }

Or will the above not work for some reason?

> Nullifying it guarantees that the code starting with "if (! STRINGP 
> (default_directory))" is used.  Maybe I should have put in a comment explaining 
> that.

Sure, but this loses information.  Of course, if we cannot do better
reliably, it's an okay solution.

Thanks.

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Andreas Schwab]

On Jul 07 2019, Ken Brown <kbrown@cornell.edu> wrote:

> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -804,7 +804,11 @@ DEFUN ("expand-file-name", Fexpand_file_name, 
> Sexpand_file_name, 1, 2, 0,
>
>     /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
>     if (NILP (default_directory))
> -    default_directory = BVAR (current_buffer, directory);
> +    {
> +      default_directory = BVAR (current_buffer, directory);
> +      if (NILP (Ffile_name_absolute_p (default_directory)))

This will signal an error if default-directory is not a string.

> +       default_directory = Qnil;
> +    }
>     if (! STRINGP (default_directory))

Just move the test here.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/8/2019 9:59 AM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown@cornell.edu>
>> CC: "npostavs@gmail.com" <npostavs@gmail.com>, "dan@dpsutton.com"
>> 	<dan@dpsutton.com>, "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>
>> Date: Mon, 8 Jul 2019 13:36:38 +0000
>>
>>>>       if (NILP (default_directory))
>>>> -    default_directory = BVAR (current_buffer, directory);
>>>> +    {
>>>> +      default_directory = BVAR (current_buffer, directory);
>>>> +      if (NILP (Ffile_name_absolute_p (default_directory)))
>>>> +       default_directory = Qnil;
>>>> +    }
>>>
>>> Hmm... why nullify it?  Why not simply call expand-file-name
>>> recursively?
>>
>> If the current buffer's default-directory is not absolute, then that variable is
>> invalid and we can't use it.
> 
> I'm asking why not do this instead:
> 
>    if (NILP (default_directory))
>      {
>        default_directory = BVAR (current_buffer, directory);
>        if (NILP (Ffile_name_absolute_p (default_directory)))
>          default_directory = Fexpand_file_name (default_directory,
> 	                                       Vinvocation_directory);
>      }

Oh, I see.  I misunderstood what you were suggesting.

> Or will the above not work for some reason?

I think something like this should work, with some care.  First, 
invocation-directory might be nil, so we have to avoid an infinite loop in that 
case.  Second, the code in emacs.c that sets Vinvocation_directory calls 
Fexpand_file_name in some cases, so there's another potential infinite loop 
resulting from that.

I'll see what I can come up with, also taking Andreas's comment into account.

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/8/2019 11:17 AM, Ken Brown wrote:
> On 7/8/2019 9:59 AM, Eli Zaretskii wrote:
>> I'm asking why not do this instead:
>>
>>     if (NILP (default_directory))
>>       {
>>         default_directory = BVAR (current_buffer, directory);
>>         if (NILP (Ffile_name_absolute_p (default_directory)))
>>           default_directory = Fexpand_file_name (default_directory,
>> 	                                       Vinvocation_directory);
>>       }
> 
> Oh, I see.  I misunderstood what you were suggesting.
> 
>> Or will the above not work for some reason?
> 
> I think something like this should work, with some care.  First,
> invocation-directory might be nil, so we have to avoid an infinite loop in that
> case.  Second, the code in emacs.c that sets Vinvocation_directory calls
> Fexpand_file_name in some cases, so there's another potential infinite loop
> resulting from that.
> 
> I'll see what I can come up with, also taking Andreas's comment into account.

New attempt:

--- a/src/fileio.c
+++ b/src/fileio.c
@@ -804,7 +804,22 @@ DEFUN ("expand-file-name", Fexpand_file_name, 
Sexpand_file_name, 1, 2, 0,

    /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
    if (NILP (default_directory))
-    default_directory = BVAR (current_buffer, directory);
+    {
+      Lisp_Object dir = BVAR (current_buffer, directory);
+      /* The buffer's default-directory should be absolute.  If it
+        isn't, try to expand it relative to invocation-directory.
+        But we have to be careful to avoid an infinite loop, because
+        the code in emacs.c that sets Vinvocation_directory might
+        call Fexpand_file_name.  */
+      if (STRINGP (dir))
+       {
+         if (!NILP (Ffile_name_absolute_p (dir)))
+           default_directory = dir;
+         else if (STRINGP (Vinvocation_directory)
+                  && !NILP (Ffile_name_absolute_p (Vinvocation_directory)))
+           default_directory = Fexpand_file_name (dir, Vinvocation_directory);
+       }
+    }
    if (! STRINGP (default_directory))
      {
  #ifdef DOS_NT

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> CC: "dan@dpsutton.com" <dan@dpsutton.com>, "36502@debbugs.gnu.org"
> 	<36502@debbugs.gnu.org>, "npostavs@gmail.com" <npostavs@gmail.com>, Andreas
>  Schwab <schwab@suse.de>
> Date: Mon, 8 Jul 2019 16:44:23 +0000
> 
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -804,7 +804,22 @@ DEFUN ("expand-file-name", Fexpand_file_name, 
> Sexpand_file_name, 1, 2, 0,
> 
>     /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
>     if (NILP (default_directory))
> -    default_directory = BVAR (current_buffer, directory);
> +    {
> +      Lisp_Object dir = BVAR (current_buffer, directory);
> +      /* The buffer's default-directory should be absolute.  If it
> +        isn't, try to expand it relative to invocation-directory.
> +        But we have to be careful to avoid an infinite loop, because
> +        the code in emacs.c that sets Vinvocation_directory might
> +        call Fexpand_file_name.  */
> +      if (STRINGP (dir))
> +       {
> +         if (!NILP (Ffile_name_absolute_p (dir)))
> +           default_directory = dir;
> +         else if (STRINGP (Vinvocation_directory)
> +                  && !NILP (Ffile_name_absolute_p (Vinvocation_directory)))
> +           default_directory = Fexpand_file_name (dir, Vinvocation_directory);
> +       }
> +    }
>     if (! STRINGP (default_directory))
>       {
>   #ifdef DOS_NT

LGTM, thanks.  Can we have a test for this subtle use case, so that we
never regress?

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/8/2019 1:23 PM, Eli Zaretskii wrote:
> LGTM, thanks.  Can we have a test for this subtle use case, so that we
> never regress?

I just found another case where expand-file-name can yield a non-absolute 
result.  Assuming there is no user "foo", we have

(let ((default-directory "~foo"))
   (expand-file-name "bar"))
=> "~foo/~foo/bar"

This is a ridiculous result, in addition to not being absolute.

Part of what's confusing here is that file-name-absolute-p returns t on file 
names starting with "~", even though its doc string explicitly states that such 
a file name is not absolute.

My inclination is that if default-directory starts with "~", then we should try 
to convert it to a (truly) absolute file name.  If this doesn't work, then we 
should forget the special interpretation of "~" and just treat default-directory 
the same as any other relative name.  This means we would get 
"<invocation-directory>/~foo" if invocation-directory is absolute, and "/~foo" 
otherwise.

Does this seem reasonable?  Or are there other suggestions?

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > (let ((default-directory "~foo"))
  >    (expand-file-name "bar"))
  > => "~foo/~foo/bar"

That is not right, but I think "~foo/bar" would be right.

  > Part of what's confusing here is that file-name-absolute-p returns
  > t on file names starting with "~", even though its doc string
  > explicitly states that such a file name is not absolute.

This contradiction is not good.  But I would like to point out
that, in a certain sesne, a name starting with ~ is absolute.
It is not relative to the current directory.

We may have a problem with two different meanings of "absolute",
each being pertinent to some situations.

  > My inclination is that if default-directory starts with "~", then we should try 
  > to convert it to a (truly) absolute file name.

There may be some circumstances where that is better, but not always.



-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: eliz@gnu.org, dan@dpsutton.com, npostavs@gmail.com,
> 	36502@debbugs.gnu.org, monnier@iro.umontreal.ca,
> 	schwab@suse.de
> Date: Thu, 11 Jul 2019 19:36:38 -0400
> 
>   > (let ((default-directory "~foo"))
>   >    (expand-file-name "bar"))
>   > => "~foo/~foo/bar"
> 
> That is not right, but I think "~foo/bar" would be right.

No, I think this should yield an absolute file name starting with a
slash (on Posix systems).

>   > Part of what's confusing here is that file-name-absolute-p returns
>   > t on file names starting with "~", even though its doc string
>   > explicitly states that such a file name is not absolute.
> 
> This contradiction is not good.  But I would like to point out
> that, in a certain sesne, a name starting with ~ is absolute.
> It is not relative to the current directory.

It is only absolute if what follows ~ is a slash or a name of an
existing user.  I think we should fix the inconsistency in that
direction.

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

On 7/12/2019 2:41 AM, Eli Zaretskii wrote:
>> From: Richard Stallman <rms@gnu.org>
>> Cc: eliz@gnu.org, dan@dpsutton.com, npostavs@gmail.com,
>> 	36502@debbugs.gnu.org, monnier@iro.umontreal.ca,
>> 	schwab@suse.de
>> Date: Thu, 11 Jul 2019 19:36:38 -0400
>>
>>    > (let ((default-directory "~foo"))
>>    >    (expand-file-name "bar"))
>>    > => "~foo/~foo/bar"
>>
>> That is not right, but I think "~foo/bar" would be right.
> 
> No, I think this should yield an absolute file name starting with a
> slash (on Posix systems).
> 
>>    > Part of what's confusing here is that file-name-absolute-p returns
>>    > t on file names starting with "~", even though its doc string
>>    > explicitly states that such a file name is not absolute.
>>
>> This contradiction is not good.  But I would like to point out
>> that, in a certain sesne, a name starting with ~ is absolute.
>> It is not relative to the current directory.
> 
> It is only absolute if what follows ~ is a slash or a name of an
> existing user.  I think we should fix the inconsistency in that
> direction.

Patch attached.

Ken

[-- Attachment #2: fix_tilde.patch --]
[-- Type: text/plain, Size: 3533 bytes --]

diff --git a/src/fileio.c b/src/fileio.c
index 614c0f989d..d9167c135c 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -737,6 +737,13 @@ DEFUN ("make-temp-name", Fmake_temp_name, Smake_temp_name, 1, 1, 0,
 				   empty_unibyte_string, Qnil);
 }
 
+/* NAME must be a string.  */
+static bool
+file_name_absolute_no_tilde_p (Lisp_Object name)
+{
+  return IS_ABSOLUTE_FILE_NAME (SSDATA (name));
+}
+
 DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
        doc: /* Convert filename NAME to absolute, and canonicalize it.
 Second arg DEFAULT-DIRECTORY is directory to start with if NAME is relative
@@ -801,41 +808,54 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
       error ("Invalid handler in `file-name-handler-alist'");
     }
 
+  /* As a last resort, we may have to use the root as
+     default_directory below.  */
+  Lisp_Object root;
+#ifdef DOS_NT
+      /* "/" is not considered a root directory on DOS_NT, so using it
+	 as default_directory causes an infinite recursion in, e.g.,
+	 the following:
+
+            (let (default-directory)
+	      (expand-file-name "a"))
+
+	 To avoid this, we use the root of the current drive.  */
+      root = build_string (emacs_root_dir ());
+#else
+      root = build_string ("/");
+#endif
 
   /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
   if (NILP (default_directory))
     {
       Lisp_Object dir = BVAR (current_buffer, directory);
-      /* The buffer's default-directory should be absolute.  If it
-	 isn't, try to expand it relative to invocation-directory.
-	 But we have to be careful to avoid an infinite loop, because
-	 the code in emacs.c that sets Vinvocation_directory might
-	 call Fexpand_file_name.  */
+      /* The buffer's default-directory should be absolute or should
+	 start with `~'.  If it isn't absolute, we replace it by its
+	 expansion relative to a known absolute name ABSDIR, which is
+	 the invocation-directory if the latter is absolute, or the
+	 root otherwise.
+
+	 In case default-directory starts with `~' or `~user', where
+	 USER is a valid user name, this correctly expands it (and
+	 ABSDIR plays no role).  If USER is not a valid user name, the
+	 leading `~' loses its special meaning and is retained as part
+	 of the expanded name.  */
       if (STRINGP (dir))
 	{
-	  if (!NILP (Ffile_name_absolute_p (dir)))
+	  if (file_name_absolute_no_tilde_p (dir))
 	    default_directory = dir;
-	  else if (STRINGP (Vinvocation_directory)
-		   && !NILP (Ffile_name_absolute_p (Vinvocation_directory)))
-	    default_directory = Fexpand_file_name (dir, Vinvocation_directory);
+	  else
+	    {
+	      Lisp_Object absdir
+		= STRINGP (Vinvocation_directory)
+		&& file_name_absolute_no_tilde_p (Vinvocation_directory)
+		? Vinvocation_directory : root;
+	      default_directory = Fexpand_file_name (dir, absdir);
+	    }
 	}
     }
   if (! STRINGP (default_directory))
-    {
-#ifdef DOS_NT
-      /* "/" is not considered a root directory on DOS_NT, so using "/"
-	 here causes an infinite recursion in, e.g., the following:
-
-            (let (default-directory)
-	      (expand-file-name "a"))
-
-	 To avoid this, we set default_directory to the root of the
-	 current drive.  */
-      default_directory = build_string (emacs_root_dir ());
-#else
-      default_directory = build_string ("/");
-#endif
-    }
+    default_directory = root;
 
   handler = Ffind_file_name_handler (default_directory, Qexpand_file_name);
   if (!NILP (handler))

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

On 7/12/2019 4:18 PM, Ken Brown wrote:
> On 7/12/2019 2:41 AM, Eli Zaretskii wrote:
>> It is only absolute if what follows ~ is a slash or a name of an
>> existing user.  I think we should fix the inconsistency in that
>> direction.
> 
> Patch attached.

Is the patch OK, Eli?  Here it is again, fleshed out with a commit message and test.

Ken

[-- Attachment #2: 0001-Fix-expand-file-name-for-names-starting-with.patch --]
[-- Type: text/plain, Size: 4952 bytes --]

From bdc13b7b64b2f1314b920ab57e6986d4cf866d14 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Mon, 15 Jul 2019 09:32:49 -0400
Subject: [PATCH] Fix expand-file-name for names starting with '~'

* src/fileio.c: (Fexpand_file_name): If the current buffer's
default-directory starts with "~user" where "user" is not a valid
user name, don't give the '~' a special meaning.  Just treat the
value of default-directory as a relative name.  (Bug#36502)
* test/src/fileio-tests.el
(fileio-tests--relative-default-directory): Add a test.
---
 src/fileio.c             | 68 ++++++++++++++++++++++++++--------------
 test/src/fileio-tests.el |  6 +++-
 2 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index 7f83267956..4c7625cad4 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -737,6 +737,13 @@ DEFUN ("make-temp-name", Fmake_temp_name, Smake_temp_name, 1, 1, 0,
 				   empty_unibyte_string, Qnil);
 }
 
+/* NAME must be a string.  */
+static bool
+file_name_absolute_no_tilde_p (Lisp_Object name)
+{
+  return IS_ABSOLUTE_FILE_NAME (SSDATA (name));
+}
+
 DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
        doc: /* Convert filename NAME to absolute, and canonicalize it.
 Second arg DEFAULT-DIRECTORY is directory to start with if NAME is relative
@@ -807,41 +814,54 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
       error ("Invalid handler in `file-name-handler-alist'");
     }
 
+  /* As a last resort, we may have to use the root as
+     default_directory below.  */
+  Lisp_Object root;
+#ifdef DOS_NT
+      /* "/" is not considered a root directory on DOS_NT, so using it
+	 as default_directory causes an infinite recursion in, e.g.,
+	 the following:
+
+            (let (default-directory)
+	      (expand-file-name "a"))
+
+	 To avoid this, we use the root of the current drive.  */
+      root = build_string (emacs_root_dir ());
+#else
+      root = build_string ("/");
+#endif
 
   /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
   if (NILP (default_directory))
     {
       Lisp_Object dir = BVAR (current_buffer, directory);
-      /* The buffer's default-directory should be absolute.  If it
-	 isn't, try to expand it relative to invocation-directory.
-	 But we have to be careful to avoid an infinite loop, because
-	 the code in emacs.c that sets Vinvocation_directory might
-	 call Fexpand_file_name.  */
+      /* The buffer's default-directory should be absolute or should
+	 start with `~'.  If it isn't absolute, we replace it by its
+	 expansion relative to a known absolute name ABSDIR, which is
+	 the invocation-directory if the latter is absolute, or the
+	 root otherwise.
+
+	 In case default-directory starts with `~' or `~user', where
+	 USER is a valid user name, this correctly expands it (and
+	 ABSDIR plays no role).  If USER is not a valid user name, the
+	 leading `~' loses its special meaning and is retained as part
+	 of the expanded name.  */
       if (STRINGP (dir))
 	{
-	  if (!NILP (Ffile_name_absolute_p (dir)))
+	  if (file_name_absolute_no_tilde_p (dir))
 	    default_directory = dir;
-	  else if (STRINGP (Vinvocation_directory)
-		   && !NILP (Ffile_name_absolute_p (Vinvocation_directory)))
-	    default_directory = Fexpand_file_name (dir, Vinvocation_directory);
+	  else
+	    {
+	      Lisp_Object absdir
+		= STRINGP (Vinvocation_directory)
+		&& file_name_absolute_no_tilde_p (Vinvocation_directory)
+		? Vinvocation_directory : root;
+	      default_directory = Fexpand_file_name (dir, absdir);
+	    }
 	}
     }
   if (! STRINGP (default_directory))
-    {
-#ifdef DOS_NT
-      /* "/" is not considered a root directory on DOS_NT, so using "/"
-	 here causes an infinite recursion in, e.g., the following:
-
-            (let (default-directory)
-	      (expand-file-name "a"))
-
-	 To avoid this, we set default_directory to the root of the
-	 current drive.  */
-      default_directory = build_string (emacs_root_dir ());
-#else
-      default_directory = build_string ("/");
-#endif
-    }
+    default_directory = root;
 
   handler = Ffind_file_name_handler (default_directory, Qexpand_file_name);
   if (!NILP (handler))
diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index 0e0230a145..813ee5f798 100644
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -131,4 +131,8 @@ fileio-tests--insert-file-interrupt
 (ert-deftest fileio-tests--relative-default-directory ()
   "Test expand-file-name when default-directory is relative."
   (let ((default-directory "some/relative/name"))
-    (should (file-name-absolute-p (expand-file-name "foo")))))
+    (should (file-name-absolute-p (expand-file-name "foo"))))
+  (let* ((default-directory "~foo")
+         (name (expand-file-name "bar")))
+    (should (and (file-name-absolute-p name)
+                 (not (eq (aref name 0) ?~))))))
-- 
2.21.0

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> CC: "dan@dpsutton.com" <dan@dpsutton.com>, "36502@debbugs.gnu.org"
> 	<36502@debbugs.gnu.org>, "npostavs@gmail.com" <npostavs@gmail.com>,
> 	"monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>, "schwab@suse.de"
> 	<schwab@suse.de>
> Date: Mon, 15 Jul 2019 13:39:24 +0000
> 
> On 7/12/2019 4:18 PM, Ken Brown wrote:
> > On 7/12/2019 2:41 AM, Eli Zaretskii wrote:
> >> It is only absolute if what follows ~ is a slash or a name of an
> >> existing user.  I think we should fix the inconsistency in that
> >> direction.
> > 
> > Patch attached.
> 
> Is the patch OK, Eli?  Here it is again, fleshed out with a commit message and test.

Sorry for the delay, Ken.  I was traveling last week, and had a large
backlog to wade through upon my return.  I'm almost done with that
now, and will review this shortly.

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> CC: "dan@dpsutton.com" <dan@dpsutton.com>, "36502@debbugs.gnu.org"
> 	<36502@debbugs.gnu.org>, "npostavs@gmail.com" <npostavs@gmail.com>,
> 	"monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>, "schwab@suse.de"
> 	<schwab@suse.de>
> Date: Mon, 15 Jul 2019 13:39:24 +0000
> 
> On 7/12/2019 4:18 PM, Ken Brown wrote:
> > On 7/12/2019 2:41 AM, Eli Zaretskii wrote:
> >> It is only absolute if what follows ~ is a slash or a name of an
> >> existing user.  I think we should fix the inconsistency in that
> >> direction.
> > 
> > Patch attached.
> 
> Is the patch OK, Eli?  Here it is again, fleshed out with a commit message and test.

Thanks.  The patch looks OK to me, but shouldn't we also make
file-name-absolute-p recognize "~foo" as non-absolute when there's no
user named "foo"?  I thought we agreed this is a discrepancy we don't
want.

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/20/2019 5:19 AM, Eli Zaretskii wrote:
>> From: Ken Brown <kbrown@cornell.edu>
>> CC: "dan@dpsutton.com" <dan@dpsutton.com>, "36502@debbugs.gnu.org"
>> 	<36502@debbugs.gnu.org>, "npostavs@gmail.com" <npostavs@gmail.com>,
>> 	"monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>, "schwab@suse.de"
>> 	<schwab@suse.de>
>> Date: Mon, 15 Jul 2019 13:39:24 +0000
>>
>> On 7/12/2019 4:18 PM, Ken Brown wrote:
>>> On 7/12/2019 2:41 AM, Eli Zaretskii wrote:
>>>> It is only absolute if what follows ~ is a slash or a name of an
>>>> existing user.  I think we should fix the inconsistency in that
>>>> direction.
>>>
>>> Patch attached.
>>
>> Is the patch OK, Eli?  Here it is again, fleshed out with a commit message and test.
> 
> Thanks.  The patch looks OK to me, but shouldn't we also make
> file-name-absolute-p recognize "~foo" as non-absolute when there's no
> user named "foo"?  I thought we agreed this is a discrepancy we don't
> want.

I'm not sure.  The current behavior is longstanding and was explicitly 
documented by Paul (added to the CC) in the last couple years.  Might there be 
some code that relies on this behavior?

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> CC: "rms@gnu.org" <rms@gnu.org>, "dan@dpsutton.com" <dan@dpsutton.com>,
> 	"36502@debbugs.gnu.org" <36502@debbugs.gnu.org>, "npostavs@gmail.com"
> 	<npostavs@gmail.com>, "monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>,
> 	"schwab@suse.de" <schwab@suse.de>, Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 20 Jul 2019 14:27:08 +0000
> 
> > Thanks.  The patch looks OK to me, but shouldn't we also make
> > file-name-absolute-p recognize "~foo" as non-absolute when there's no
> > user named "foo"?  I thought we agreed this is a discrepancy we don't
> > want.
> 
> I'm not sure.  The current behavior is longstanding and was explicitly 
> documented by Paul (added to the CC) in the last couple years.  Might there be 
> some code that relies on this behavior?

I'd be surprised.  Paul, can you comment, please?

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Paul Eggert]

Ken Brown wrote:
>> The patch looks OK to me, but shouldn't we also make
>> file-name-absolute-p recognize "~foo" as non-absolute when there's no
>> user named "foo"?  I thought we agreed this is a discrepancy we don't
>> want.
> I'm not sure.  The current behavior is longstanding and was explicitly
> documented by Paul (added to the CC) in the last couple years.  Might there be
> some code that relies on this behavior?

As I recall, I documented it because of the confusion encountered when dealing 
with Bug#28156 (Emacs quietly munges symlink contents).

I looked into the history of file-name-absolute-p back to 1991. Although the 
current behavior is indeed longstanding, I don't think it was originally 
intended to treat ~foo/x as absolute when "foo" is not a valid username. I think 
the code was originally written under the assumption that this case was not 
worth worrying about. My impression from looking at uses of file-name-absolute-p 
is that changing it to return nil here would improve correctness of the callers, 
though there would be a performance cost for this case of course.

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Eli Zaretskii]

> Cc: "rms@gnu.org" <rms@gnu.org>, "dan@dpsutton.com" <dan@dpsutton.com>,
>  "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>,
>  "npostavs@gmail.com" <npostavs@gmail.com>,
>  "monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>,
>  "schwab@suse.de" <schwab@suse.de>
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 20 Jul 2019 19:32:30 -0700
> 
> My impression from looking at uses of file-name-absolute-p 
> is that changing it to return nil here would improve correctness of the callers, 
> though there would be a performance cost for this case of course.

I agree.

So Ken, please push your changes, and I would appreciate if you could
also make the change in file-name-absolute-p (as a separate change),
if you have time.

Thanks.

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/21/2019 10:21 AM, Eli Zaretskii wrote:
>> Cc: "rms@gnu.org" <rms@gnu.org>, "dan@dpsutton.com" <dan@dpsutton.com>,
>>   "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>,
>>   "npostavs@gmail.com" <npostavs@gmail.com>,
>>   "monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>,
>>   "schwab@suse.de" <schwab@suse.de>
>> From: Paul Eggert <eggert@cs.ucla.edu>
>> Date: Sat, 20 Jul 2019 19:32:30 -0700
>>
>> My impression from looking at uses of file-name-absolute-p
>> is that changing it to return nil here would improve correctness of the callers,
>> though there would be a performance cost for this case of course.
> 
> I agree.
> 
> So Ken, please push your changes, and I would appreciate if you could
> also make the change in file-name-absolute-p (as a separate change),
> if you have time.

OK, I'll work on that.

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

On 7/21/2019 10:21 AM, Eli Zaretskii wrote:
>> Cc: "rms@gnu.org" <rms@gnu.org>, "dan@dpsutton.com" <dan@dpsutton.com>,
>>   "36502@debbugs.gnu.org" <36502@debbugs.gnu.org>,
>>   "npostavs@gmail.com" <npostavs@gmail.com>,
>>   "monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>,
>>   "schwab@suse.de" <schwab@suse.de>
>> From: Paul Eggert <eggert@cs.ucla.edu>
>> Date: Sat, 20 Jul 2019 19:32:30 -0700
>>
>> My impression from looking at uses of file-name-absolute-p
>> is that changing it to return nil here would improve correctness of the callers,
>> though there would be a performance cost for this case of course.
> 
> I agree.
> 
> So Ken, please push your changes, and I would appreciate if you could
> also make the change in file-name-absolute-p (as a separate change),
> if you have time.

Patch attached.  I didn't add a NEWS entry.  Do you think this requires one?

Ken

[-- Attachment #2: 0001-Fix-file-name-absolute-p-for-names-starting-with.patch --]
[-- Type: text/plain, Size: 6435 bytes --]

From 3657bc708e96c6955f64e111daf52c3c3667a6db Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Sun, 21 Jul 2019 22:04:07 -0400
Subject: [PATCH] Fix file-name-absolute-p for names starting with '~'

A file name starting with "~user" is now considered absolute only
if "user" is a valid login name.  See the discussion starting at
Bug#36502#64.
* src/fileio.c (expand_tilde): New static function, extracted from
Fexpand_file_name.
(Fexpand_file_name, file_name_absolute_p)
(search_embedded_absfilename):  Use it.
(Ffile_name_absolute_p): Update doc string.
* doc/lispref/files.texi (Relative File Names): Document the new
behavior of file-name-absolute-p.
---
 doc/lispref/files.texi | 10 ++++-
 src/fileio.c           | 84 ++++++++++++++++++++++++++++--------------
 2 files changed, 65 insertions(+), 29 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 0519f787dc..18325b2049 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2154,7 +2154,11 @@ Relative File Names
 
 @defun file-name-absolute-p filename
 This function returns @code{t} if file @var{filename} is an absolute
-file name or begins with @samp{~}, @code{nil} otherwise.
+file name or begins with @samp{~}, provided that an initial
+@samp{~@var{user}} corresponds to a valid login name @var{user}.  The
+function returns @code{nil} otherwise.  In the following examples,
+assume that there is a user named @samp{rms} but no user named
+@samp{quux}.
 
 @example
 @group
@@ -2162,6 +2166,10 @@ Relative File Names
      @result{} t
 @end group
 @group
+(file-name-absolute-p "~quux/foo")
+     @result{} nil
+@end group
+@group
 (file-name-absolute-p "rms/foo")
      @result{} nil
 @end group
diff --git a/src/fileio.c b/src/fileio.c
index 4c7625cad4..42455bb499 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -744,6 +744,35 @@ file_name_absolute_no_tilde_p (Lisp_Object name)
   return IS_ABSOLUTE_FILE_NAME (SSDATA (name));
 }
 
+/* NAME must start with "~user", where USER is not empty, followed by
+   nothing or a directory separator.  LENGTH is the length of the
+   "~user" prefix.  Return the absolute file name of USER's home
+   directory, or Qnil if there is no user named USER.  */
+static Lisp_Object
+expand_tilde (const char *name, ptrdiff_t length)
+{
+  char *p;
+  const char *dir;
+  struct passwd *pw;
+  Lisp_Object result;
+  USE_SAFE_ALLOCA;
+
+  p = SAFE_ALLOCA (length);
+  memcpy (p, name + 1, length - 1);
+  p[length] = 0;
+
+  pw = getpwnam (p);
+  if (pw)
+    {
+      dir = pw->pw_dir;
+      result = make_unibyte_string (dir, strlen (dir));
+    }
+  else
+    result = Qnil;
+  SAFE_FREE ();
+  return result;
+}
+
 DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
        doc: /* Convert filename NAME to absolute, and canonicalize it.
 Second arg DEFAULT-DIRECTORY is directory to start with if NAME is relative
@@ -788,7 +817,6 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
   char *target;
 
   ptrdiff_t tlen;
-  struct passwd *pw;
 #ifdef DOS_NT
   int drive = 0;
   bool collapse_newdir = true;
@@ -1153,25 +1181,20 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
 	}
       else			/* ~user/filename */
 	{
-	  char *o, *p;
+	  char *p;
 	  for (p = nm; *p && !IS_DIRECTORY_SEP (*p); p++)
 	    continue;
-	  o = SAFE_ALLOCA (p - nm + 1);
-	  memcpy (o, nm, p - nm);
-	  o[p - nm] = 0;
 
 	  block_input ();
-	  pw = getpwnam (o + 1);
+	  Lisp_Object tem = expand_tilde (nm, p - nm);
 	  unblock_input ();
-	  if (pw)
+	  if (!NILP (tem))
 	    {
-	      Lisp_Object tem;
-
-	      newdir = pw->pw_dir;
-	      /* `getpwnam' may return a unibyte string, which will
-		 bite us when we expect the directory to be multibyte.  */
-	      tem = make_unibyte_string (newdir, strlen (newdir));
+	      newdir = SSDATA (tem);
 	      newdirlim = newdir + SBYTES (tem);
+	      /* `getpwnam', which was used in `expand_tilde', may
+		 return a unibyte string, which will bite us when we
+		 expect the directory to be multibyte.  */
 	      if (multibyte && !STRING_MULTIBYTE (tem))
 		{
 		  hdir = DECODE_FILE (tem);
@@ -1670,13 +1693,23 @@ DEAFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
 bool
 file_name_absolute_p (const char *filename)
 {
-  return
-    (IS_DIRECTORY_SEP (*filename) || *filename == '~'
+  bool result
+    = (IS_DIRECTORY_SEP (*filename)
 #ifdef DOS_NT
-     || (IS_DRIVE (*filename) && IS_DEVICE_SEP (filename[1])
-	 && IS_DIRECTORY_SEP (filename[2]))
+       || (IS_DRIVE (*filename) && IS_DEVICE_SEP (filename[1])
+	   && IS_DIRECTORY_SEP (filename[2]))
 #endif
      );
+  if (!result && *filename == '~')
+    {
+      const char *p;
+      for (p = filename + 1; *p && !IS_DIRECTORY_SEP (*p); p++);
+      if (p == filename + 1)
+	result = true;
+      else
+	result = !NILP (expand_tilde (filename, p - filename));
+    }
+  return result;
 }
 
 /* Put into BUF the concatenation of DIR and FILE, with an intervening
@@ -1794,20 +1827,13 @@ search_embedded_absfilename (char *nm, char *endp)
 	  for (s = p; *s && !IS_DIRECTORY_SEP (*s); s++);
 	  if (p[0] == '~' && s > p + 1)	/* We've got "/~something/".  */
 	    {
-	      USE_SAFE_ALLOCA;
-	      char *o = SAFE_ALLOCA (s - p + 1);
-	      struct passwd *pw;
-	      memcpy (o, p, s - p);
-	      o [s - p] = 0;
-
 	      /* If we have ~user and `user' exists, discard
 		 everything up to ~.  But if `user' does not exist, leave
 		 ~user alone, it might be a literal file name.  */
 	      block_input ();
-	      pw = getpwnam (o + 1);
+	      Lisp_Object tem = expand_tilde (p, s - p);
 	      unblock_input ();
-	      SAFE_FREE ();
-	      if (pw)
+	      if (!NILP (tem))
 		return p;
 	    }
 	  else
@@ -2698,8 +2724,10 @@ DEFUN ("make-symbolic-link", Fmake_symbolic_link, Smake_symbolic_link, 2, 3,
 \f
 DEFUN ("file-name-absolute-p", Ffile_name_absolute_p, Sfile_name_absolute_p,
        1, 1, 0,
-       doc: /* Return t if FILENAME is an absolute file name or starts with `~'.
-On Unix, absolute file names start with `/'.  */)
+       doc: /* Return t if FILENAME is an absolute file name.
+On Unix, absolute file names are usually required to start with `/';
+but here we also allow FILENAME to start with `~', provided that an
+initial ~USER corresponds to a valid login name USER.  */)
   (Lisp_Object filename)
 {
   CHECK_STRING (filename);
-- 
2.21.0

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 194 bytes --]

Thanks for the patch. I improved it, wrote a NEWS entry and a test case for it, 
and installed the attached. I think this finishes off the issues in Bug#36502 
and so am closing the bug report.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-treat-nosuchuser-as-an-absolute-file-name.patch --]
[-- Type: text/x-patch; name="0001-Do-not-treat-nosuchuser-as-an-absolute-file-name.patch", Size: 9392 bytes --]

From a5063aa8b174db286a0e83b8ffdd4e65c521f733 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 24 Jul 2019 14:28:13 -0700
Subject: [PATCH] Do not treat ~nosuchuser as an absolute file name
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Derived from Ken Brown’s patch (Bug#36502#97).
* doc/lispref/files.texi (Relative File Names):
* etc/NEWS: Document this.
* src/fileio.c (user_homedir): New function.
(Fexpand_file_name, file_name_absolute_p): Use it.
(search_embedded_absfilename): Simplify via file_name_absolute_p.
* test/src/fileio-tests.el (fileio-tests--no-such-user): New test.
---
 doc/lispref/files.texi   |  10 +++-
 etc/NEWS                 |   3 +
 src/fileio.c             | 121 ++++++++++++++++++---------------------
 test/src/fileio-tests.el |  11 ++++
 4 files changed, 78 insertions(+), 67 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 0519f787dc..0ea8a4f0a1 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2154,7 +2154,11 @@ Relative File Names
 
 @defun file-name-absolute-p filename
 This function returns @code{t} if file @var{filename} is an absolute
-file name or begins with @samp{~}, @code{nil} otherwise.
+file name, @code{nil} otherwise.  A file name is considered to be
+absolute if its first component is @samp{~}, or is @samp{~@var{user}}
+where @var{user} is a valid login name.  In the following examples,
+assume that there is a user named @samp{rms} but no user named
+@samp{nosuchuser}.
 
 @example
 @group
@@ -2162,6 +2166,10 @@ Relative File Names
      @result{} t
 @end group
 @group
+(file-name-absolute-p "~nosuchuser/foo")
+     @result{} nil
+@end group
+@group
 (file-name-absolute-p "rms/foo")
      @result{} nil
 @end group
diff --git a/etc/NEWS b/etc/NEWS
index 5313270411..08f0e654f7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1815,6 +1815,9 @@ relative to the 'default-directory' of the current buffer.  We recommend
 always setting "$HOME" to an absolute file name, so that its meaning is
 independent of where Emacs was started.
 
+** file-name-absolute-p no longer considers "~foo" to be an absolute
+file name if there is no user named "foo".
+
 ** The FILENAME argument to 'file-name-base' is now mandatory and no
 longer defaults to 'buffer-file-name'.
 
diff --git a/src/fileio.c b/src/fileio.c
index e4269b96a3..d1a7f39ac9 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -744,6 +744,31 @@ file_name_absolute_no_tilde_p (Lisp_Object name)
   return IS_ABSOLUTE_FILE_NAME (SSDATA (name));
 }
 
+/* Return the home directory of the user NAME, or a null pointer if
+   NAME is empty or the user does not exist or the user's home
+   directory is not an absolute file name.  NAME is an array of bytes
+   that continues up to (but not including) the next NUL byte or
+   directory separator.  The returned string lives in storage good
+   until the next call to this or similar functions.  */
+static char *
+user_homedir (char const *name)
+{
+  ptrdiff_t length;
+  for (length = 0; name[length] && !IS_DIRECTORY_SEP (name[length]); length++)
+    continue;
+  if (length == 0)
+    return NULL;
+  USE_SAFE_ALLOCA;
+  char *p = SAFE_ALLOCA (length + 1);
+  memcpy (p, name, length);
+  p[length] = 0;
+  struct passwd *pw = getpwnam (p);
+  SAFE_FREE ();
+  if (!pw || (pw->pw_dir && !IS_ABSOLUTE_FILE_NAME (pw->pw_dir)))
+    return NULL;
+  return pw->pw_dir;
+}
+
 DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
        doc: /* Convert filename NAME to absolute, and canonicalize it.
 Second arg DEFAULT-DIRECTORY is directory to start with if NAME is relative
@@ -788,7 +813,6 @@ the root directory.  */)
   char *target;
 
   ptrdiff_t tlen;
-  struct passwd *pw;
 #ifdef DOS_NT
   int drive = 0;
   bool collapse_newdir = true;
@@ -1153,39 +1177,29 @@ the root directory.  */)
 	}
       else			/* ~user/filename */
 	{
-	  char *o, *p;
-	  for (p = nm; *p && !IS_DIRECTORY_SEP (*p); p++)
-	    continue;
-	  o = SAFE_ALLOCA (p - nm + 1);
-	  memcpy (o, nm, p - nm);
-	  o[p - nm] = 0;
-
-	  block_input ();
-	  pw = getpwnam (o + 1);
-	  unblock_input ();
-	  if (pw)
+	  char *nmhome = user_homedir (nm + 1);
+	  if (nmhome)
 	    {
-	      Lisp_Object tem;
-
-	      newdir = pw->pw_dir;
-	      /* `getpwnam' may return a unibyte string, which will
-		 bite us when we expect the directory to be multibyte.  */
-	      tem = make_unibyte_string (newdir, strlen (newdir));
-	      newdirlim = newdir + SBYTES (tem);
-	      if (multibyte && !STRING_MULTIBYTE (tem))
+	      ptrdiff_t nmhomelen = strlen (nmhome);
+	      newdir = nmhome;
+	      newdirlim = newdir + nmhomelen;
+	      if (multibyte)
 		{
-		  hdir = DECODE_FILE (tem);
+		  AUTO_STRING_WITH_LEN (lisp_nmhome, nmhome, nmhomelen);
+		  hdir = DECODE_FILE (lisp_nmhome);
 		  newdir = SSDATA (hdir);
 		  newdirlim = newdir + SBYTES (hdir);
 		}
-	      nm = p;
+
+	      while (*++nm && !IS_DIRECTORY_SEP (*nm))
+		continue;
 #ifdef DOS_NT
 	      collapse_newdir = false;
 #endif
 	    }
 
 	  /* If we don't find a user of that name, leave the name
-	     unchanged; don't move nm forward to p.  */
+	     unchanged.  */
 	}
     }
 
@@ -1667,18 +1681,6 @@ See also the function `substitute-in-file-name'.")
 }
 #endif
 \f
-bool
-file_name_absolute_p (const char *filename)
-{
-  return
-    (IS_DIRECTORY_SEP (*filename) || *filename == '~'
-#ifdef DOS_NT
-     || (IS_DRIVE (*filename) && IS_DEVICE_SEP (filename[1])
-	 && IS_DIRECTORY_SEP (filename[2]))
-#endif
-     );
-}
-
 /* Put into BUF the concatenation of DIR and FILE, with an intervening
    directory separator if needed.  Return a pointer to the NUL byte
    at the end of the concatenated string.  */
@@ -1774,7 +1776,10 @@ get_homedir (void)
   return ahome;
 }
 
-/* If /~ or // appears, discard everything through first slash.  */
+/* If a directory separator followed by an absolute file name (e.g.,
+   "//foo", "/~", "/~someuser") appears in NM, return the address of
+   the absolute file name.  Otherwise return NULL.  ENDP is the
+   address of the null byte at the end of NM.  */
 static char *
 search_embedded_absfilename (char *nm, char *endp)
 {
@@ -1784,34 +1789,8 @@ search_embedded_absfilename (char *nm, char *endp)
 	&& !IS_DIRECTORY_SEP (p[1]));
 #endif
   for (; p < endp; p++)
-    {
-      if (IS_DIRECTORY_SEP (p[-1]) && file_name_absolute_p (p))
-	{
-	  char *s;
-	  for (s = p; *s && !IS_DIRECTORY_SEP (*s); s++)
-	    continue;
-	  if (p[0] == '~' && s > p + 1)	/* We've got "/~something/".  */
-	    {
-	      USE_SAFE_ALLOCA;
-	      char *o = SAFE_ALLOCA (s - p + 1);
-	      struct passwd *pw;
-	      memcpy (o, p, s - p);
-	      o [s - p] = 0;
-
-	      /* If we have ~user and `user' exists, discard
-		 everything up to ~.  But if `user' does not exist, leave
-		 ~user alone, it might be a literal file name.  */
-	      block_input ();
-	      pw = getpwnam (o + 1);
-	      unblock_input ();
-	      SAFE_FREE ();
-	      if (pw)
-		return p;
-	    }
-	  else
-	    return p;
-	}
-    }
+    if (IS_DIRECTORY_SEP (p[-1]) && file_name_absolute_p (p))
+      return p;
   return NULL;
 }
 
@@ -2696,13 +2675,23 @@ This happens for interactive use with M-x.  */)
 \f
 DEFUN ("file-name-absolute-p", Ffile_name_absolute_p, Sfile_name_absolute_p,
        1, 1, 0,
-       doc: /* Return t if FILENAME is an absolute file name or starts with `~'.
-On Unix, absolute file names start with `/'.  */)
+       doc: /* Return t if FILENAME is an absolute file name.
+On Unix, absolute file names start with `/'.  In Emacs, an absolute
+file name can also start with an initial `~' or `~USER' component,
+where USER is a valid login name.  */)
   (Lisp_Object filename)
 {
   CHECK_STRING (filename);
   return file_name_absolute_p (SSDATA (filename)) ? Qt : Qnil;
 }
+
+bool
+file_name_absolute_p (char const *filename)
+{
+  return (IS_ABSOLUTE_FILE_NAME (filename)
+	  || (filename[0] == '~'
+	      && (!filename[1] || user_homedir (&filename[1]))));
+}
 \f
 DEFUN ("file-exists-p", Ffile_exists_p, Sfile_exists_p, 1, 1, 0,
        doc: /* Return t if file FILENAME exists (whether or not you can read it).
diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index 813ee5f798..09a5b147e1 100644
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -136,3 +136,14 @@ fileio-tests--symlink-failure
          (name (expand-file-name "bar")))
     (should (and (file-name-absolute-p name)
                  (not (eq (aref name 0) ?~))))))
+
+(ert-deftest fileio-tests--no-such-user ()
+  "Test file-name-absolute-p on ~nosuchuser."
+  (unless (user-full-name "nosuchuser")
+    (should (not (file-name-absolute-p "~nosuchuser")))
+    (should (not (file-name-absolute-p "~nosuchuser/")))
+    (should (not (file-name-absolute-p "~nosuchuser//")))
+    (should (not (file-name-absolute-p "~nosuchuser/foo")))
+    (should (not (file-name-absolute-p "~nosuchuser/foo/")))
+    (should (not (file-name-absolute-p "~nosuchuser/foo//")))
+    (should (not (file-name-absolute-p "~nosuchuser/foo/bar")))))
-- 
2.17.1

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Ken Brown]

On 7/24/2019 5:36 PM, Paul Eggert wrote:
> Thanks for the patch. I improved it, wrote a NEWS entry and a test case for it, 
> and installed the attached. I think this finishes off the issues in Bug#36502 
> and so am closing the bug report.

Thanks.  I like your improvements.

Ken

bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

[Andy Moreton]

On Wed 24 Jul 2019, Paul Eggert wrote:

> Thanks for the patch. I improved it, wrote a NEWS entry and a test case for
> it, and installed the attached. I think this finishes off the issues in
> Bug#36502 and so am closing the bug report.

This patch causes a regression in UI behaviour, tested with 64bit MSYS2
build on Windows. Bisection shows this patch to be the cause. Before
this patch, from emacs -Q:

   C-x C-f ~/foo RET
   => opens existing directory "c:/home/ajm/foo/" in dired

   C-x C-f ~/bar RET
   => default path offered is "~/foo"
      entered path is appended to give "~/foo/~/bar"
      opens existing directory "c:/home/ajm/bar/" in dired

After this patch the second find-file fails and opens an empty buffer.
The echo area shows:

  Use M-x make-directory RET RET to create the directory and its parents

`buffer-file-name' in that buffer is invalid: "c:/home/ajm/foo/~/bar".

Please fix this.

    AndyM