* bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p @ 2019-07-04 16:48 Daniel Sutton 2019-07-04 18:08 ` Ken Brown 0 siblings, 1 reply; 34+ messages in thread From: Daniel Sutton @ 2019-07-04 16:48 UTC (permalink / raw) To: 36502 [-- 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 --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-04 16:48 bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p Daniel Sutton @ 2019-07-04 18:08 ` Ken Brown [not found] ` <CAMfzp7bhcmeY7QP4-ALfmBE4OojJthcYEVLR79zj-FrGx5s+WA@mail.gmail.com> 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-04 18:08 UTC (permalink / raw) To: Daniel Sutton, 36502@debbugs.gnu.org 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CAMfzp7bhcmeY7QP4-ALfmBE4OojJthcYEVLR79zj-FrGx5s+WA@mail.gmail.com>]
* bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p [not found] ` <CAMfzp7bhcmeY7QP4-ALfmBE4OojJthcYEVLR79zj-FrGx5s+WA@mail.gmail.com> @ 2019-07-05 1:32 ` Ken Brown [not found] ` <CAMfzp7brsFLdpi04pDAL+O_yVuF7=EERzinVBKoQyTaLUtgwDA@mail.gmail.com> 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-05 1:32 UTC (permalink / raw) To: Daniel Sutton; +Cc: 36502@debbugs.gnu.org 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CAMfzp7brsFLdpi04pDAL+O_yVuF7=EERzinVBKoQyTaLUtgwDA@mail.gmail.com>]
[parent not found: <CAMfzp7Y=wA8_V=Tvm1iOtyXM-kqKZyx41Q4phJfnwmygHhJWLA@mail.gmail.com>]
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p [not found] ` <CAMfzp7Y=wA8_V=Tvm1iOtyXM-kqKZyx41Q4phJfnwmygHhJWLA@mail.gmail.com> @ 2019-07-05 3:05 ` Daniel Sutton 2019-07-06 12:49 ` Ken Brown 2019-07-06 13:27 ` Noam Postavsky 0 siblings, 2 replies; 34+ messages in thread From: Daniel Sutton @ 2019-07-05 3:05 UTC (permalink / raw) To: 36502 [-- 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 --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-05 3:05 ` bug#36502: Fwd: " Daniel Sutton @ 2019-07-06 12:49 ` Ken Brown 2019-07-06 13:27 ` Noam Postavsky 1 sibling, 0 replies; 34+ messages in thread From: Ken Brown @ 2019-07-06 12:49 UTC (permalink / raw) To: Daniel Sutton, 36502-done@debbugs.gnu.org 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-05 3:05 ` bug#36502: Fwd: " Daniel Sutton 2019-07-06 12:49 ` Ken Brown @ 2019-07-06 13:27 ` Noam Postavsky 2019-07-06 15:38 ` Ken Brown 1 sibling, 1 reply; 34+ messages in thread From: Noam Postavsky @ 2019-07-06 13:27 UTC (permalink / raw) To: Daniel Sutton; +Cc: 36502 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-06 13:27 ` Noam Postavsky @ 2019-07-06 15:38 ` Ken Brown 2019-07-06 16:14 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-06 15:38 UTC (permalink / raw) To: Noam Postavsky, Daniel Sutton; +Cc: 36502@debbugs.gnu.org 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-06 15:38 ` Ken Brown @ 2019-07-06 16:14 ` Eli Zaretskii 2019-07-07 14:09 ` Ken Brown 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2019-07-06 16:14 UTC (permalink / raw) To: Ken Brown; +Cc: dan, npostavs, 36502 > 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? ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-06 16:14 ` Eli Zaretskii @ 2019-07-07 14:09 ` Ken Brown 2019-07-07 14:37 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-07 14:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dan@dpsutton.com, npostavs@gmail.com, 36502@debbugs.gnu.org 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-07 14:09 ` Ken Brown @ 2019-07-07 14:37 ` Eli Zaretskii 2019-07-07 19:30 ` Ken Brown 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2019-07-07 14:37 UTC (permalink / raw) To: Ken Brown; +Cc: dan, npostavs, 36502 > 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? ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-07 14:37 ` Eli Zaretskii @ 2019-07-07 19:30 ` Ken Brown 2019-07-08 12:25 ` Eli Zaretskii 2019-07-08 14:37 ` Andreas Schwab 0 siblings, 2 replies; 34+ messages in thread From: Ken Brown @ 2019-07-07 19:30 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dan@dpsutton.com, npostavs@gmail.com, 36502@debbugs.gnu.org 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-07 19:30 ` Ken Brown @ 2019-07-08 12:25 ` Eli Zaretskii 2019-07-08 13:36 ` Ken Brown 2019-07-08 14:37 ` Andreas Schwab 1 sibling, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2019-07-08 12:25 UTC (permalink / raw) To: Ken Brown; +Cc: dan, npostavs, 36502 > 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? ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-08 12:25 ` Eli Zaretskii @ 2019-07-08 13:36 ` Ken Brown 2019-07-08 13:59 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-08 13:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: dan@dpsutton.com, npostavs@gmail.com, 36502@debbugs.gnu.org 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-08 13:36 ` Ken Brown @ 2019-07-08 13:59 ` Eli Zaretskii 2019-07-08 15:17 ` Ken Brown 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2019-07-08 13:59 UTC (permalink / raw) To: Ken Brown; +Cc: dan, npostavs, 36502 > 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-08 13:59 ` Eli Zaretskii @ 2019-07-08 15:17 ` Ken Brown 2019-07-08 16:44 ` Ken Brown 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-08 15:17 UTC (permalink / raw) To: Eli Zaretskii Cc: dan@dpsutton.com, 36502@debbugs.gnu.org, npostavs@gmail.com, Andreas Schwab 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-08 15:17 ` Ken Brown @ 2019-07-08 16:44 ` Ken Brown 2019-07-08 17:23 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-08 16:44 UTC (permalink / raw) To: Eli Zaretskii Cc: dan@dpsutton.com, npostavs@gmail.com, 36502@debbugs.gnu.org, Andreas Schwab 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-08 16:44 ` Ken Brown @ 2019-07-08 17:23 ` Eli Zaretskii 2019-07-10 21:57 ` Ken Brown 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2019-07-08 17:23 UTC (permalink / raw) To: Ken Brown; +Cc: dan, npostavs, 36502, schwab > 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? ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-08 17:23 ` Eli Zaretskii @ 2019-07-10 21:57 ` Ken Brown 2019-07-11 23:36 ` Richard Stallman 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-10 21:57 UTC (permalink / raw) To: Eli Zaretskii Cc: dan@dpsutton.com, npostavs@gmail.com, 36502@debbugs.gnu.org, Stefan Monnier, schwab@suse.de 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-10 21:57 ` Ken Brown @ 2019-07-11 23:36 ` Richard Stallman 2019-07-12 6:41 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Richard Stallman @ 2019-07-11 23:36 UTC (permalink / raw) To: Ken Brown; +Cc: dan, npostavs, 36502, monnier, schwab [[[ 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) ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-11 23:36 ` Richard Stallman @ 2019-07-12 6:41 ` Eli Zaretskii 2019-07-12 20:18 ` Ken Brown 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2019-07-12 6:41 UTC (permalink / raw) To: rms; +Cc: schwab, npostavs, 36502, monnier, dan > 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-12 6:41 ` Eli Zaretskii @ 2019-07-12 20:18 ` Ken Brown 2019-07-15 13:39 ` Ken Brown 0 siblings, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-12 20:18 UTC (permalink / raw) To: Eli Zaretskii, rms@gnu.org Cc: dan@dpsutton.com, 36502@debbugs.gnu.org, npostavs@gmail.com, monnier@iro.umontreal.ca, schwab@suse.de [-- 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)) ^ permalink raw reply related [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-12 20:18 ` Ken Brown @ 2019-07-15 13:39 ` Ken Brown 2019-07-19 7:00 ` Eli Zaretskii 2019-07-20 9:19 ` Eli Zaretskii 0 siblings, 2 replies; 34+ messages in thread From: Ken Brown @ 2019-07-15 13:39 UTC (permalink / raw) To: Eli Zaretskii, rms@gnu.org Cc: dan@dpsutton.com, npostavs@gmail.com, 36502@debbugs.gnu.org, monnier@iro.umontreal.ca, schwab@suse.de [-- 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-15 13:39 ` Ken Brown @ 2019-07-19 7:00 ` Eli Zaretskii 2019-07-20 9:19 ` Eli Zaretskii 1 sibling, 0 replies; 34+ messages in thread From: Eli Zaretskii @ 2019-07-19 7:00 UTC (permalink / raw) To: Ken Brown; +Cc: 36502 > 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-15 13:39 ` Ken Brown 2019-07-19 7:00 ` Eli Zaretskii @ 2019-07-20 9:19 ` Eli Zaretskii 2019-07-20 14:27 ` Ken Brown 1 sibling, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2019-07-20 9:19 UTC (permalink / raw) To: Ken Brown; +Cc: rms, schwab, 36502, npostavs, monnier, dan > 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-20 9:19 ` Eli Zaretskii @ 2019-07-20 14:27 ` Ken Brown 2019-07-20 15:52 ` Eli Zaretskii 2019-07-21 2:32 ` Paul Eggert 0 siblings, 2 replies; 34+ messages in thread From: Ken Brown @ 2019-07-20 14:27 UTC (permalink / raw) To: Eli Zaretskii Cc: Paul Eggert, rms@gnu.org, schwab@suse.de, 36502@debbugs.gnu.org, npostavs@gmail.com, monnier@iro.umontreal.ca, dan@dpsutton.com 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-20 14:27 ` Ken Brown @ 2019-07-20 15:52 ` Eli Zaretskii 2019-07-21 2:32 ` Paul Eggert 1 sibling, 0 replies; 34+ messages in thread From: Eli Zaretskii @ 2019-07-20 15:52 UTC (permalink / raw) To: Ken Brown; +Cc: eggert, 36502 > 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? ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-20 14:27 ` Ken Brown 2019-07-20 15:52 ` Eli Zaretskii @ 2019-07-21 2:32 ` Paul Eggert 2019-07-21 14:21 ` Eli Zaretskii 1 sibling, 1 reply; 34+ messages in thread From: Paul Eggert @ 2019-07-21 2:32 UTC (permalink / raw) To: Ken Brown, Eli Zaretskii Cc: rms@gnu.org, schwab@suse.de, 36502@debbugs.gnu.org, npostavs@gmail.com, monnier@iro.umontreal.ca, dan@dpsutton.com 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-21 2:32 ` Paul Eggert @ 2019-07-21 14:21 ` Eli Zaretskii 2019-07-21 14:30 ` Ken Brown 2019-07-22 2:16 ` Ken Brown 0 siblings, 2 replies; 34+ messages in thread From: Eli Zaretskii @ 2019-07-21 14:21 UTC (permalink / raw) To: Paul Eggert; +Cc: rms, dan, 36502, npostavs, monnier, schwab > 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-21 14:21 ` Eli Zaretskii @ 2019-07-21 14:30 ` Ken Brown 2019-07-22 2:16 ` Ken Brown 1 sibling, 0 replies; 34+ messages in thread From: Ken Brown @ 2019-07-21 14:30 UTC (permalink / raw) To: Eli Zaretskii, Paul Eggert Cc: rms@gnu.org, schwab@suse.de, 36502@debbugs.gnu.org, npostavs@gmail.com, monnier@iro.umontreal.ca, dan@dpsutton.com 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-21 14:21 ` Eli Zaretskii 2019-07-21 14:30 ` Ken Brown @ 2019-07-22 2:16 ` Ken Brown 2019-07-24 21:36 ` Paul Eggert 1 sibling, 1 reply; 34+ messages in thread From: Ken Brown @ 2019-07-22 2:16 UTC (permalink / raw) To: Eli Zaretskii, Paul Eggert Cc: rms@gnu.org, schwab@suse.de, 36502@debbugs.gnu.org, npostavs@gmail.com, monnier@iro.umontreal.ca, dan@dpsutton.com [-- 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-22 2:16 ` Ken Brown @ 2019-07-24 21:36 ` Paul Eggert 2019-07-24 22:47 ` Ken Brown 2019-07-26 11:04 ` Andy Moreton 0 siblings, 2 replies; 34+ messages in thread From: Paul Eggert @ 2019-07-24 21:36 UTC (permalink / raw) To: Ken Brown, Eli Zaretskii Cc: rms@gnu.org, schwab@suse.de, 36502-done@debbugs.gnu.org, npostavs@gmail.com, monnier@iro.umontreal.ca, dan@dpsutton.com [-- 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-24 21:36 ` Paul Eggert @ 2019-07-24 22:47 ` Ken Brown 2019-07-26 11:04 ` Andy Moreton 1 sibling, 0 replies; 34+ messages in thread From: Ken Brown @ 2019-07-24 22:47 UTC (permalink / raw) To: Paul Eggert, Eli Zaretskii Cc: rms@gnu.org, schwab@suse.de, 36502-done@debbugs.gnu.org, npostavs@gmail.com, monnier@iro.umontreal.ca, dan@dpsutton.com 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-24 21:36 ` Paul Eggert 2019-07-24 22:47 ` Ken Brown @ 2019-07-26 11:04 ` Andy Moreton 1 sibling, 0 replies; 34+ messages in thread From: Andy Moreton @ 2019-07-26 11:04 UTC (permalink / raw) To: 36502 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p 2019-07-07 19:30 ` Ken Brown 2019-07-08 12:25 ` Eli Zaretskii @ 2019-07-08 14:37 ` Andreas Schwab 1 sibling, 0 replies; 34+ messages in thread From: Andreas Schwab @ 2019-07-08 14:37 UTC (permalink / raw) To: Ken Brown; +Cc: dan@dpsutton.com, npostavs@gmail.com, 36502@debbugs.gnu.org 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." ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2019-07-26 11:04 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-04 16:48 bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p Daniel Sutton
2019-07-04 18:08 ` Ken Brown
[not found] ` <CAMfzp7bhcmeY7QP4-ALfmBE4OojJthcYEVLR79zj-FrGx5s+WA@mail.gmail.com>
2019-07-05 1:32 ` Ken Brown
[not found] ` <CAMfzp7brsFLdpi04pDAL+O_yVuF7=EERzinVBKoQyTaLUtgwDA@mail.gmail.com>
[not found] ` <CAMfzp7Y=wA8_V=Tvm1iOtyXM-kqKZyx41Q4phJfnwmygHhJWLA@mail.gmail.com>
2019-07-05 3:05 ` bug#36502: Fwd: " Daniel Sutton
2019-07-06 12:49 ` Ken Brown
2019-07-06 13:27 ` Noam Postavsky
2019-07-06 15:38 ` Ken Brown
2019-07-06 16:14 ` Eli Zaretskii
2019-07-07 14:09 ` Ken Brown
2019-07-07 14:37 ` Eli Zaretskii
2019-07-07 19:30 ` Ken Brown
2019-07-08 12:25 ` Eli Zaretskii
2019-07-08 13:36 ` Ken Brown
2019-07-08 13:59 ` Eli Zaretskii
2019-07-08 15:17 ` Ken Brown
2019-07-08 16:44 ` Ken Brown
2019-07-08 17:23 ` Eli Zaretskii
2019-07-10 21:57 ` Ken Brown
2019-07-11 23:36 ` Richard Stallman
2019-07-12 6:41 ` Eli Zaretskii
2019-07-12 20:18 ` Ken Brown
2019-07-15 13:39 ` Ken Brown
2019-07-19 7:00 ` Eli Zaretskii
2019-07-20 9:19 ` Eli Zaretskii
2019-07-20 14:27 ` Ken Brown
2019-07-20 15:52 ` Eli Zaretskii
2019-07-21 2:32 ` Paul Eggert
2019-07-21 14:21 ` Eli Zaretskii
2019-07-21 14:30 ` Ken Brown
2019-07-22 2:16 ` Ken Brown
2019-07-24 21:36 ` Paul Eggert
2019-07-24 22:47 ` Ken Brown
2019-07-26 11:04 ` Andy Moreton
2019-07-08 14:37 ` Andreas Schwab
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).