bug#27871: 26.0.50; Bad handling of unmounted directory

bug#27871: 26.0.50; Bad handling of unmounted directory

[Philipp]

Create a FUSE mount point, e.g.

cd /tmp
mkdir a a/a b
touch a/a/a
bindfs -f a b

While bindfs(1) is running, cd to the mounted directory in a second
shell:

cd /tmp/b/a

Now kill bindfs, e.g. by hitting ^C in the first shell.  The second
shell will now be in an unmounted directory.  From that directory, start
Emacs:

$ emacs -Q -batch a
Apparent cycle of symbolic links for (unreachable)

or, with stack trace

$ emacs -Q -batch -f toggle-debug-on-error a
Debug on Error enabled globally
...
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)/a/(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a/(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)/a/(unreachable)/a/(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a/(unreachable)/a/(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)/a/(unreachable)/a/(unreachable)/a/(unreachable)" (-1) (nil))
  file-truename("(unreachable)/a/(unreachable)/a/(unreachable)/a/(unreachable)/a" (-1) (nil))
  file-truename("(unreachable)/a/(unreachable)/a/(unreachable)/a/(unreachable)/a/a")
  find-file-noselect("(unreachable)/a/(unreachable)/a/a")
  #f(compiled-function (displayable-buffers dir line column name) #<bytecode>)((nil) "(unreachable)/a/" (0) (0) "a")
  command-line-1(("-f" "toggle-debug-on-error" "a"))
  command-line()
  normal-top-level()

It seems like the logic for `default-directory' and/or `file-truename'
should be improved for unmounted filesystems.


In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.10.8)
 of 2017-07-29 built on unknown
Repository revision: 2c930d15f541761422a268cd2b5a7f5c11c9a00e
Windowing system distributor 'The X.Org Foundation', version 11.0.11803000
System Description:	Ubuntu 14.04.5 LTS

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --with-modules --without-pop --with-mailutils
 --enable-checking --enable-check-lisp-object-type --enable-gcc-warnings
 'CFLAGS=-ggdb3 -O0''

Configured features:
XPM JPEG TIFF GIF PNG SOUND GSETTINGS NOTIFY GNUTLS FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-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
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date mule-util
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 inotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 94115 11926)
 (symbols 48 20077 1)
 (miscs 40 38 119)
 (strings 32 28435 1945)
 (string-bytes 1 754739)
 (vectors 16 13893)
 (vector-slots 8 487465 12899)
 (floats 8 48 97)
 (intervals 56 205 0)
 (buffers 992 11)
 (heap 1024 19129 984))

bug#27871: 26.0.50; Bad handling of unmounted directory

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 3131 bytes --]

Philipp <p.stephani2@gmail.com> schrieb am Sa., 29. Juli 2017 um 23:08 Uhr:

>
> Create a FUSE mount point, e.g.
>
> cd /tmp
> mkdir a a/a b
> touch a/a/a
> bindfs -f a b
>
> While bindfs(1) is running, cd to the mounted directory in a second
> shell:
>
> cd /tmp/b/a
>
> Now kill bindfs, e.g. by hitting ^C in the first shell.  The second
> shell will now be in an unmounted directory.  From that directory, start
> Emacs:
>
> $ emacs -Q -batch a
> Apparent cycle of symbolic links for (unreachable)
>
> or, with stack trace
>
> $ emacs -Q -batch -f toggle-debug-on-error a
> Debug on Error enabled globally
> ...
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)/a/(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a/(unreachable)/a" (-1) (nil))
>   file-truename("(unreachable)/a/(unreachable)/a/(unreachable)" (-1) (nil))
>   file-truename("(unreachable)/a/(unreachable)/a/(unreachable)/a" (-1)
> (nil))
>
> file-truename("(unreachable)/a/(unreachable)/a/(unreachable)/a/(unreachable)"
> (-1) (nil))
>
> file-truename("(unreachable)/a/(unreachable)/a/(unreachable)/a/(unreachable)/a"
> (-1) (nil))
>
> file-truename("(unreachable)/a/(unreachable)/a/(unreachable)/a/(unreachable)/a/a")
>   find-file-noselect("(unreachable)/a/(unreachable)/a/a")
>   #f(compiled-function (displayable-buffers dir line column name)
> #<bytecode>)((nil) "(unreachable)/a/" (0) (0) "a")
>   command-line-1(("-f" "toggle-debug-on-error" "a"))
>   command-line()
>   normal-top-level()
>
> It seems like the logic for `default-directory' and/or `file-truename'
> should be improved for unmounted filesystems.
>
>
Here's a patch. With the patch, the output is

 Error getting directory: Transport endpoint is not connected
Warning (initialization): Error setting default-directory
Ignoring relative file name (a) due to nil default-directory

[-- Attachment #1.2: Type: text/html, Size: 4143 bytes --]

[-- Attachment #2: 0001-Treat-unreachable-current-directory-as-error.txt --]
[-- Type: text/plain, Size: 2118 bytes --]

From b3541b3bc936bbb8e9950d1aed14d2571e03c1fe Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sat, 23 Sep 2017 12:13:39 +0200
Subject: [PATCH] Treat unreachable current directory as error

Linux prefixes an unreachable (e.g. unmounted) current directory with
the special string "(unreachable)", cf. Bug#27871.  Treat such
directories as error because they wouldn't work anyway.

* src/sysdep.c (emacs_get_current_dir_name_1): Renamed from
emacs_get_current_dir_name.
(emacs_get_current_dir_name): Check for prefix "(unreachable)".
---
 src/sysdep.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/sysdep.c b/src/sysdep.c
index 1e6e0d011b..6c76f16242 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -220,10 +220,8 @@ init_standard_fds (void)
   force_open (STDERR_FILENO, O_RDONLY);
 }
 
-/* Return the current working directory.  The result should be freed
-   with 'free'.  Return NULL on errors.  */
-char *
-emacs_get_current_dir_name (void)
+static char *
+emacs_get_current_dir_name_1 (void)
 {
 # if HAVE_GET_CURRENT_DIR_NAME && !BROKEN_GET_CURRENT_DIR_NAME
 #  ifdef HYBRID_MALLOC
@@ -283,6 +281,29 @@ emacs_get_current_dir_name (void)
   return buf;
 }
 
+/* Return the current working directory.  The result should be freed
+   with 'free'.  Return NULL on errors.  */
+char *
+emacs_get_current_dir_name (void)
+{
+  char *dir = emacs_get_current_dir_name_1 ();
+  if (dir == NULL)
+    return NULL;
+  /* On Linux, getcwd and get_current_dir_name return a string
+     starting with "(unreachable)" if the current directory doesn't
+     exist, e.g. because it was unmounted.  Treat that as an error.
+     See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27871.  */
+  const char *prefix = "(unreachable)";
+  size_t dir_len = strlen (dir);
+  size_t prefix_len = strlen (prefix);
+  if (dir_len >= prefix_len && strncmp (dir, prefix, prefix_len) == 0)
+    {
+      errno = ENOTCONN;
+      return NULL;
+    }
+  return dir;
+}
+
 \f
 /* Discard pending input on all input descriptors.  */
 
-- 
2.14.1.821.g8fa685d3b7-goog

bug#27871: 26.0.50; Bad handling of unmounted directory

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 23 Sep 2017 10:19:19 +0000
> 
> Here's a patch. With the patch, the output is
> 
> Error getting directory: Transport endpoint is not connected
> Warning (initialization): Error setting default-directory
> Ignoring relative file name (a) due to nil default-directory

Thanks.

> +/* Return the current working directory.  The result should be freed
> +   with 'free'.  Return NULL on errors.  */
> +char *
> +emacs_get_current_dir_name (void)
> +{
> +  char *dir = emacs_get_current_dir_name_1 ();
> +  if (dir == NULL)
> +    return NULL;
> +  /* On Linux, getcwd and get_current_dir_name return a string
> +     starting with "(unreachable)" if the current directory doesn't
> +     exist, e.g. because it was unmounted.  Treat that as an error.
> +     See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27871.  */
> +  const char *prefix = "(unreachable)";
> +  size_t dir_len = strlen (dir);
> +  size_t prefix_len = strlen (prefix);
> +  if (dir_len >= prefix_len && strncmp (dir, prefix, prefix_len) == 0)
> +    {
> +      errno = ENOTCONN;
> +      return NULL;

What if there's a directory called literally "(unreachable)SOMETHING"?
This isn't disallowed on GNU/Linux, is it?  We should probably stat
the result if we see this prefix, before we declare it a failure, IMO.

bug#27871: 26.0.50; Bad handling of unmounted directory

[Andreas Schwab]

On Sep 23 2017, Eli Zaretskii <eliz@gnu.org> wrote:

>> +/* Return the current working directory.  The result should be freed
>> +   with 'free'.  Return NULL on errors.  */
>> +char *
>> +emacs_get_current_dir_name (void)
>> +{
>> +  char *dir = emacs_get_current_dir_name_1 ();
>> +  if (dir == NULL)
>> +    return NULL;
>> +  /* On Linux, getcwd and get_current_dir_name return a string
>> +     starting with "(unreachable)" if the current directory doesn't
>> +     exist, e.g. because it was unmounted.  Treat that as an error.
>> +     See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27871.  */
>> +  const char *prefix = "(unreachable)";
>> +  size_t dir_len = strlen (dir);
>> +  size_t prefix_len = strlen (prefix);
>> +  if (dir_len >= prefix_len && strncmp (dir, prefix, prefix_len) == 0)
>> +    {
>> +      errno = ENOTCONN;
>> +      return NULL;
>
> What if there's a directory called literally "(unreachable)SOMETHING"?

An absolute file name cannot start with "(unreachable)".

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#27871: 26.0.50; Bad handling of unmounted directory

[Philipp Stephani]

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

Andreas Schwab <schwab@linux-m68k.org> schrieb am Sa., 23. Sep. 2017 um
13:30 Uhr:

> On Sep 23 2017, Eli Zaretskii <eliz@gnu.org> wrote:
>
> >> +/* Return the current working directory.  The result should be freed
> >> +   with 'free'.  Return NULL on errors.  */
> >> +char *
> >> +emacs_get_current_dir_name (void)
> >> +{
> >> +  char *dir = emacs_get_current_dir_name_1 ();
> >> +  if (dir == NULL)
> >> +    return NULL;
> >> +  /* On Linux, getcwd and get_current_dir_name return a string
> >> +     starting with "(unreachable)" if the current directory doesn't
> >> +     exist, e.g. because it was unmounted.  Treat that as an error.
> >> +     See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27871.  */
> >> +  const char *prefix = "(unreachable)";
> >> +  size_t dir_len = strlen (dir);
> >> +  size_t prefix_len = strlen (prefix);
> >> +  if (dir_len >= prefix_len && strncmp (dir, prefix, prefix_len) == 0)
> >> +    {
> >> +      errno = ENOTCONN;
> >> +      return NULL;
> >
> > What if there's a directory called literally "(unreachable)SOMETHING"?
>
> An absolute file name cannot start with "(unreachable)".
>
>
Yes, and getcwd and friends only return absolute filenames, and we only use
$PWD if it's absolute, so anything except '/' or a drive letter can't be a
prefix in the success case. I'll add a comment to that effect.

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

bug#27871: 26.0.50; Bad handling of unmounted directory

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 23 Sep 2017 11:33:13 +0000
> Cc: 27871@debbugs.gnu.org
> 
>  > What if there's a directory called literally "(unreachable)SOMETHING"?
> 
>  An absolute file name cannot start with "(unreachable)".
> 
> Yes, and getcwd and friends only return absolute filenames, and we only use $PWD if it's absolute, so
> anything except '/' or a drive letter can't be a prefix in the success case. I'll add a comment to that effect. 

Then why not just check that the first character is something other
than '/'?

bug#27871: 26.0.50; Bad handling of unmounted directory

[Eli Zaretskii]

> From: Eli Zaretskii <eliz@gnu.org>
> Cc: schwab@linux-m68k.org, 27871@debbugs.gnu.org
> 
> Then why not just check that the first character is something other
> than '/'?

Or, more general, that the result fails file_name_absolute_p (which
would be more portable)?

bug#27871: 26.0.50; Bad handling of unmounted directory

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 23. Sep. 2017 um 13:41 Uhr:

> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: schwab@linux-m68k.org, 27871@debbugs.gnu.org
> >
> > Then why not just check that the first character is something other
> > than '/'?
>
> Or, more general, that the result fails file_name_absolute_p (which
> would be more portable)?
>

Sounds reasonable, I'll send a new patch.

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

bug#27871: [PATCH] Treat unreachable current directory as error

[Philipp Stephani]

Linux prefixes an unreachable (e.g. unmounted) current directory with
the special string "(unreachable)", cf. Bug#27871.  Treat such
directories as error because they wouldn't work anyway.

* src/sysdep.c (emacs_get_current_dir_name_1): Renamed from
emacs_get_current_dir_name.
(emacs_get_current_dir_name): Check for prefix "(unreachable)".
---
 src/sysdep.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/sysdep.c b/src/sysdep.c
index 1e6e0d011b..efc0396c93 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -220,10 +220,8 @@ init_standard_fds (void)
   force_open (STDERR_FILENO, O_RDONLY);
 }
 
-/* Return the current working directory.  The result should be freed
-   with 'free'.  Return NULL on errors.  */
-char *
-emacs_get_current_dir_name (void)
+static char *
+emacs_get_current_dir_name_1 (void)
 {
 # if HAVE_GET_CURRENT_DIR_NAME && !BROKEN_GET_CURRENT_DIR_NAME
 #  ifdef HYBRID_MALLOC
@@ -283,6 +281,27 @@ emacs_get_current_dir_name (void)
   return buf;
 }
 
+/* Return the current working directory.  The result should be freed
+   with 'free'.  Return NULL on errors.  */
+char *
+emacs_get_current_dir_name (void)
+{
+  char *dir = emacs_get_current_dir_name_1 ();
+  if (dir == NULL)
+    return NULL;
+  /* On Linux, getcwd and get_current_dir_name return a string
+     starting with "(unreachable)" if the current directory doesn't
+     exist, e.g. because it was unmounted.  Treat that as an error.
+     See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27871.  */
+  if (!file_name_absolute_p (dir))
+    {
+      free (dir);
+      errno = ENOTCONN;
+      return NULL;
+    }
+  return dir;
+}
+
 \f
 /* Discard pending input on all input descriptors.  */
 
-- 
2.14.1

bug#27871: [PATCH] Treat unreachable current directory as error

[Paul Eggert]

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

Philipp Stephani wrote:

> +  if (!file_name_absolute_p (dir))

That doesn't look right here, since leading '~' counts as absolute to 
file_name_absolute_p, which is not what is wanted here.

> +      errno = ENOTCONN;

Why ENOTCONN? Shouldn't it be ENOENT? The failure has nothing to do with socket 
connections.

Also, I'd feel a bit better if we apply the workaround only to the function that 
has the problem.

How about the attached patch instead?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-bug-with-unmounted-directory-on-GNU-Linux.patch --]
[-- Type: text/x-patch; name="0001-Fix-bug-with-unmounted-directory-on-GNU-Linux.patch", Size: 1077 bytes --]

From a1f839b0c60fb345944ee39626e8645402c1f060 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 30 Sep 2017 16:53:54 -0700
Subject: [PATCH] Fix bug with unmounted directory on GNU/Linux

* src/sysdep.c (emacs_get_current_dir_name): Do not use
get_current_dir_name result unless it is absolute (Bug#27871).
---
 src/sysdep.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/sysdep.c b/src/sysdep.c
index 1e6e0d011b..a3526c3740 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -232,7 +232,18 @@ emacs_get_current_dir_name (void)
   bool use_libc = true;
 #  endif
   if (use_libc)
-    return get_current_dir_name ();
+    {
+      /* GNU/Linux get_current_dir_name can return a string starting
+	 with "(unreachable)" (Bug#27871).  */
+      char *wd = get_current_dir_name ();
+      if (wd && ! (IS_DIRECTORY_SEP (*wd) || (*wd && IS_DEVICE_SEP (wd[1]))))
+	{
+	  free (wd);
+	  errno = ENOENT;
+	  return NULL;
+	}
+      return wd;
+    }
 # endif
 
   char *buf;
-- 
2.13.5

bug#27871: [PATCH] Treat unreachable current directory as error

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 30 Sep 2017 17:00:06 -0700
> Cc: Philipp Stephani <phst@google.com>
> 
> How about the attached patch instead?

Thanks, this LGTM.  I think we want this on the release branch.

bug#27871: [PATCH] Treat unreachable current directory as error

[Paul Eggert]

On 10/05/2017 03:13 AM, Eli Zaretskii wrote:
> Thanks, this LGTM.  I think we want this on the release branch.

OK, thanks, installed, and closing this bug report.

bug#27871: [PATCH] Treat unreachable current directory as error

[Philipp Stephani]

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

Paul Eggert <eggert@cs.ucla.edu> schrieb am So., 1. Okt. 2017 um 02:00 Uhr:

> Philipp Stephani wrote:
>
> > +  if (!file_name_absolute_p (dir))
>
> That doesn't look right here, since leading '~' counts as absolute to
> file_name_absolute_p, which is not what is wanted here.
>

That shouldn't matter because getcwd never returns a string starting with ~.


>
> > +      errno = ENOTCONN;
>
> Why ENOTCONN? Shouldn't it be ENOENT? The failure has nothing to do with
> socket
> connections.
>

I think ENOTCONN is the error returned by stat(".") if the current
directory doesn't exist. But I don't care much about the exact error
number, any nonzero value should do the job.


>
> Also, I'd feel a bit better if we apply the workaround only to the
> function that
> has the problem.
>
>
All of the current directory functions exhibit this behavior, including
getwd and getcwd, so you need to make sure they are also covered.

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

bug#27871: [PATCH] Treat unreachable current directory as error

[Paul Eggert]

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

Philipp Stephani wrote:
> All of the current directory functions exhibit this behavior, including
> getwd and getcwd, so you need to make sure they are also covered.

Thanks for letting us know. I installed the 2nd attached patch, which addresses 
this by making the patch behave more like what you originally proposed, while 
still avoiding the need to use file_name_absolute_p (which is about Emacs file 
names, not OS names). Also, I noticed a related memory leak and fixed that by 
installing the 1st attached patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-src-xsmfns.c-x_session_initialize-Fix-memory-leak.patch --]
[-- Type: text/x-patch; name="0001-src-xsmfns.c-x_session_initialize-Fix-memory-leak.patch", Size: 933 bytes --]

From 2202952b8307f3a6407820280e94e4d979b7a122 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 7 Oct 2017 22:48:49 -0700
Subject: [PATCH 1/2] * src/xsmfns.c (x_session_initialize): Fix memory leak.

---
 src/xsmfns.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/xsmfns.c b/src/xsmfns.c
index 2cb4f3e..fb0d01b 100644
--- a/src/xsmfns.c
+++ b/src/xsmfns.c
@@ -401,12 +401,14 @@ x_session_initialize (struct x_display_info *dpyinfo)
   ptrdiff_t name_len = 0;
 
   /* libSM seems to crash if pwd is missing - see bug#18851.  */
-  if (! emacs_get_current_dir_name ())
+  char *pwd = emacs_get_current_dir_name ();
+  if (!pwd)
     {
       fprintf (stderr, "Disabling session management due to pwd error: %s\n",
                emacs_strerror (errno));
       return;
     }
+  xfree (pwd);
 
   ice_fd = -1;
   doing_interact = false;
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Improve-test-for-unreachable-dirs.patch --]
[-- Type: text/x-patch; name="0002-Improve-test-for-unreachable-dirs.patch", Size: 2460 bytes --]

From 7c2c117c91eeef5e7bd70c98cc7e201007016b1e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 7 Oct 2017 22:56:29 -0700
Subject: [PATCH 2/2] Improve test for unreachable dirs

* src/sysdep.c (get_current_dir_name_or_unreachable):
New function, with most of the old contents of
emacs_get_current_dir_name.
(emacs_get_current_dir_name): Use it.  Use a simpler
test for unreachable directory strings, and also apply
it to getcwd etc. (Bug#27871)
---
 src/sysdep.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/sysdep.c b/src/sysdep.c
index 8291a60..c348492 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -221,9 +221,12 @@ init_standard_fds (void)
 }
 
 /* Return the current working directory.  The result should be freed
-   with 'free'.  Return NULL on errors.  */
-char *
-emacs_get_current_dir_name (void)
+   with 'free'.  Return NULL (setting errno) on errors.  If the
+   current directory is unreachable, return either NULL or a string
+   beginning with '('.  */
+
+static char *
+get_current_dir_name_or_unreachable (void)
 {
 # if HAVE_GET_CURRENT_DIR_NAME && !BROKEN_GET_CURRENT_DIR_NAME
 #  ifdef HYBRID_MALLOC
@@ -233,16 +236,9 @@ emacs_get_current_dir_name (void)
 #  endif
   if (use_libc)
     {
-      /* GNU/Linux get_current_dir_name can return a string starting
-	 with "(unreachable)" (Bug#27871).  */
-      char *wd = get_current_dir_name ();
-      if (wd && ! (IS_DIRECTORY_SEP (*wd) || (*wd && IS_DEVICE_SEP (wd[1]))))
-	{
-	  free (wd);
-	  errno = ENOENT;
-	  return NULL;
-	}
-      return wd;
+      /* For an unreachable directory, this returns a string that starts
+	 with "(unreachable)"; see Bug#27871.  */
+      return get_current_dir_name ();
     }
 # endif
 
@@ -294,6 +290,23 @@ emacs_get_current_dir_name (void)
   return buf;
 }
 
+/* Return the current working directory.  The result should be freed
+   with 'free'.  Return NULL (setting errno) on errors; an unreachable
+   directory (e.g., its name starts with '(') counts as an error.  */
+
+char *
+emacs_get_current_dir_name (void)
+{
+  char *dir = get_current_dir_name_or_unreachable ();
+  if (dir && *dir == '(')
+    {
+      free (dir);
+      errno = ENOENT;
+      return NULL;
+    }
+  return dir;
+}
+
 \f
 /* Discard pending input on all input descriptors.  */
 
-- 
2.7.4

bug#27871: [PATCH] Treat unreachable current directory as error

[Philipp Stephani]

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

Paul Eggert <eggert@cs.ucla.edu> schrieb am So., 8. Okt. 2017 um 08:06 Uhr:

> Philipp Stephani wrote:
> > All of the current directory functions exhibit this behavior, including
> > getwd and getcwd, so you need to make sure they are also covered.
>
> Thanks for letting us know. I installed the 2nd attached patch, which
> addresses
> this by making the patch behave more like what you originally proposed,
> while
> still avoiding the need to use file_name_absolute_p (which is about Emacs
> file
> names, not OS names).
>

That should work, thanks. FTR, this behavior is documented in the Linux
manpage: http://man7.org/linux/man-pages/man2/getcwd.2.html

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