After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705902 - g_get_current_dir() should check PWD env var and return it if it's correct
g_get_current_dir() should check PWD env var and return it if it's correct
Product: glib
Classification: Platform
Component: general
Other FreeBSD
: Normal normal
: ---
Assigned To: gtkdev
Depends on:
Reported: 2013-08-13 12:14 UTC by Ting-Wei Lan
Modified: 2013-12-09 18:31 UTC
See Also:
GNOME target: ---
GNOME version: ---

g_get_current_dir(): consult PWD first (1.09 KB, patch)
2013-12-08 23:18 UTC, Allison Karlitskaya (desrt)
committed Details | Review
Use g_get_current_dir() (2.05 KB, patch)
2013-12-09 17:42 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Ting-Wei Lan 2013-08-13 12:14:46 UTC
get_current_dir_name() is a GNU extension, so it is not available on system without glibc.
Comment 1 Christian Persch 2013-08-13 13:28:42 UTC
get_current_dir_name() is used for a reason (as stated in the comment immediately before its use in the source code), so you'll have to furnish a reimplementation of it for non-glibc systems.
Comment 2 Ting-Wei Lan 2013-09-10 16:10:14 UTC
I found get_current_dir_name() is in io/getdirname.c of glibc source.
Comment 3 Christian Persch 2013-09-14 11:23:46 UTC
Got a patch?
Comment 4 Allison Karlitskaya (desrt) 2013-12-08 22:23:31 UTC
Comment 5 Allison Karlitskaya (desrt) 2013-12-08 22:28:41 UTC
    /* We use get_current_dir_name() here instead of getcwd / g_get_current_dir()
     * because we want to use the value from PWD (if it is correct).
     * See bug 502146.

I see.  Maybe we could modify g_get_current_dir(), then.
Comment 6 Allison Karlitskaya (desrt) 2013-12-08 22:31:32 UTC
Note: the code from glibc is this:

char *
get_current_dir_name (void)
  char *pwd;
  struct stat64 dotstat, pwdstat;

  pwd = getenv ("PWD");
  if (pwd != NULL
      && stat64 (".", &dotstat) == 0
      && stat64 (pwd, &pwdstat) == 0
      && pwdstat.st_dev == dotstat.st_dev
      && pwdstat.st_ino == dotstat.st_ino)
    /* The PWD value is correct.  Use it.  */
    return __strdup (pwd);

  return __getcwd ((char *) NULL, 0);

We could just call g_get_current_dir() instead of __getcwd() as the last line there (since getcwd(NULL) is not portable).
Comment 7 Allison Karlitskaya (desrt) 2013-12-08 23:18:56 UTC
Created attachment 263779 [details] [review]
g_get_current_dir(): consult PWD first

Check if the current directory is the same as $PWD.  This matches the
behaviour of the get_current_dir_name() function in glibc.
Comment 8 Christian Persch 2013-12-08 23:24:40 UTC
-> glib

If this gets in, I'll fix g-t to use g_get_current_dir().
Comment 9 Allison Karlitskaya (desrt) 2013-12-08 23:26:09 UTC
I only uploaded this patch for discussion.

Although, fwiw, I'd like it if GApplication worked this way too...
Comment 10 Colin Walters 2013-12-08 23:48:33 UTC
Review of attachment 263779 [details] [review]:

Should update the docs to say: Since GLib 2.40, this function will honor the "PWD" environment variable if it is set and appears correct.

While annotate will link to this bug, I'd say it's worth a comment too.

One thing we should do to be good citizens is clear PWD if we've detected it's wrong when we spawn a child process in execve().

PWD is really kind of a historical hack - the goal is to record for the user how they got to a directory, not the result of realpath() or the like.  Which matters pretty much only for Unix shells.  Sure you *can* spawn a program from Nautilus, but graphical programs don't have a visible representation of their PWD.
Comment 11 Allison Karlitskaya (desrt) 2013-12-09 17:10:28 UTC
Comment on attachment 263779 [details] [review]
g_get_current_dir(): consult PWD first

Attachment 263779 [details] pushed as a22f777 - g_get_current_dir(): consult PWD first

g-t patch on the way.
Comment 12 Allison Karlitskaya (desrt) 2013-12-09 17:42:36 UTC
Created attachment 263826 [details] [review]
Use g_get_current_dir()

get_current_dir_name() is a non-portable GNU extension.  Use
g_get_current_dir() instead, now that it does the same thing (ie:
respecting the value of PWD).
Comment 13 Christian Persch 2013-12-09 18:00:16 UTC
Comment on attachment 263826 [details] [review]
Use g_get_current_dir()

Please also bump the glib version req in configure. Thanks!
Comment 14 Allison Karlitskaya (desrt) 2013-12-09 18:31:21 UTC
Attachment 263826 [details] pushed as d52fe59 - Use g_get_current_dir()