emacs client/server mode hardcodes /tmp path

emacs client/server mode hardcodes /tmp path

[Diego 'Flameeyes' Pettenò]

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


Hi,

I'm working to make use of pam_mktemp in my systems by default,
pam_mktemp is a PAM module that creates a private temporary directory
per-user inside /tmp and sets TMP and TMPDIR accondingly.

Unfortunately emacs (server.el) and emacsclient hardcodes the path for
the socket to use to /tmp (/tmp/emacs${userid}/${servername}), rather
than checking TMPDIR (as [1] tells to do, by the way).

[1] http://www.gnu.org/prep/standards/standards.html#Semantics

-- 
Diego "Flameeyes" Pettenò
http://blog.flameeyes.eu/


[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: emacs client/server mode hardcodes /tmp path

[Stefan Monnier]

> I'm working to make use of pam_mktemp in my systems by default,
> pam_mktemp is a PAM module that creates a private temporary directory
> per-user inside /tmp and sets TMP and TMPDIR accondingly.

Indeed.  I'm not sure what should be done here.  The reason for the
hard-coded /tmp is that the emacsclient process and the emacs server
process may be executed from different contexts (different login
sessions for example), so it's not clear that they would use the same
$TMPDIR setting.

What do others think?  Should we take the risk and obey $TMPDIR?


        Stefan

Re: emacs client/server mode hardcodes /tmp path

[Diego 'Flameeyes' Pettenò]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Indeed.  I'm not sure what should be done here.  The reason for the
> hard-coded /tmp is that the emacsclient process and the emacs server
> process may be executed from different contexts (different login
> sessions for example), so it's not clear that they would use the same
> $TMPDIR setting.

If one wants to open a client for a different server it might either use
the --socket-name option or change TMPDIR for emacsclient itself.

-- 
Diego "Flameeyes" Pettenò
http://blog.flameeyes.eu/

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: emacs client/server mode hardcodes /tmp path

[Ulrich Mueller]

>>>>> On Thu, 06 Mar 2008, Stefan Monnier wrote:

>> I'm working to make use of pam_mktemp in my systems by default,
>> pam_mktemp is a PAM module that creates a private temporary
>> directory per-user inside /tmp and sets TMP and TMPDIR accondingly.

> Indeed.  I'm not sure what should be done here.  The reason for the
> hard-coded /tmp is that the emacsclient process and the emacs server
> process may be executed from different contexts (different login
> sessions for example), so it's not clear that they would use the
> same $TMPDIR setting.

> What do others think?  Should we take the risk and obey $TMPDIR?

Hi,

coming back to this old subject, has any decision been taken on this?
GNU Coding Standards [1] explicitely say:

# If you make temporary files, check the TMPDIR environment variable;
# if that variable is defined, use the specified directory instead of
# /tmp.

Ulrich

[1] http://www.gnu.org/prep/standards/standards.html#Semantics

Re: emacs client/server mode hardcodes /tmp path

[Stefan Monnier]

>>> I'm working to make use of pam_mktemp in my systems by default,
>>> pam_mktemp is a PAM module that creates a private temporary
>>> directory per-user inside /tmp and sets TMP and TMPDIR accondingly.

>> Indeed.  I'm not sure what should be done here.  The reason for the
>> hard-coded /tmp is that the emacsclient process and the emacs server
>> process may be executed from different contexts (different login
>> sessions for example), so it's not clear that they would use the
>> same $TMPDIR setting.

>> What do others think?  Should we take the risk and obey $TMPDIR?

Yes, I think we should obey TMPDIR (of course consistently between
emacsclient.c and server.el).


        Stefan

Re: emacs client/server mode hardcodes /tmp path

[Ulrich Mueller]

>>>>> On Tue, 28 Oct 2008, Stefan Monnier wrote:

> Yes, I think we should obey TMPDIR (of course consistently between
> emacsclient.c and server.el).

A patch is included below.

Ulrich


2008-10-29  Ulrich Mueller  <ulm@gentoo.org>

	* emacsclient.c (set_local_socket): Use TMPDIR (default /tmp)
	instead of hardcoded /tmp.

2008-10-29  Ulrich Mueller  <ulm@gentoo.org>

	* server.el (server-socket-dir): Use TMPDIR (default /tmp) instead
	of hardcoded /tmp.

--- lib-src/emacsclient.c	13 Oct 2008 02:32:51 -0000	1.133
+++ lib-src/emacsclient.c	29 Oct 2008 07:42:54 -0000
@@ -1128,6 +1128,7 @@
     int default_sock = !socket_name;
     int saved_errno = 0;
     char *server_name = "server";
+    char *tmpdir;
 
     if (socket_name && !index (socket_name, '/') && !index (socket_name, '\\'))
       { /* socket_name is a file name component.  */
@@ -1138,9 +1139,12 @@
 
     if (default_sock)
       {
- 	socket_name = alloca (100 + strlen (server_name));
- 	sprintf (socket_name, "/tmp/emacs%d/%s",
- 		 (int) geteuid (), server_name);
+	tmpdir = egetenv ("TMPDIR");
+	if (!tmpdir)
+	  tmpdir = "/tmp";
+	socket_name = alloca (96 + strlen (tmpdir) + strlen (server_name));
+	sprintf (socket_name, "%s/emacs%d/%s",
+		 tmpdir, (int) geteuid (), server_name);
       }
 
     if (strlen (socket_name) < sizeof (server.sun_path))
@@ -1174,9 +1178,10 @@
 	    if (pw && (pw->pw_uid != geteuid ()))
 	      {
 		/* We're running under su, apparently. */
-		socket_name = alloca (100 + strlen (server_name));
-		sprintf (socket_name, "/tmp/emacs%d/%s",
-			 (int) pw->pw_uid, server_name);
+		socket_name = alloca (96 + strlen (tmpdir)
+				      + strlen (server_name));
+		sprintf (socket_name, "%s/emacs%d/%s",
+			 tmpdir, (int) pw->pw_uid, server_name);
 
 		if (strlen (socket_name) < sizeof (server.sun_path))
 		  strcpy (server.sun_path, socket_name);
--- lisp/server.el	24 Oct 2008 09:39:32 -0000	1.169
+++ lisp/server.el	29 Oct 2008 07:42:56 -0000
@@ -202,7 +202,8 @@
 
 (defvar server-name "server")
 
-(defvar server-socket-dir (format "/tmp/emacs%d" (user-uid))
+(defvar server-socket-dir
+  (format "%s/emacs%d" (or (getenv "TMPDIR") "/tmp") (user-uid))
   "The directory in which to place the server socket.")
 
 (defun server-clients-with (property value)

Re: emacs client/server mode hardcodes /tmp path

[Juanma Barranquero]

On Wed, Oct 29, 2008 at 08:51, Ulrich Mueller <ulm@gentoo.org> wrote:

> -       socket_name = alloca (100 + strlen (server_name));
> -       sprintf (socket_name, "/tmp/emacs%d/%s",
> -                (int) geteuid (), server_name);
> +       tmpdir = egetenv ("TMPDIR");
> +       if (!tmpdir)
> +         tmpdir = "/tmp";
> +       socket_name = alloca (96 + strlen (tmpdir) + strlen (server_name));
> +       sprintf (socket_name, "%s/emacs%d/%s",
> +                tmpdir, (int) geteuid (), server_name);

There was neither much point in allocating an extra 100 bytes, nor in
allocating 96 now. The formatted uid is not going to be 88 characters
long.

  Juanma

Re: emacs client/server mode hardcodes /tmp path

[Ulrich Mueller]

>>>>> On Wed, 29 Oct 2008, Juanma Barranquero wrote:

>> +       socket_name = alloca (96 + strlen (tmpdir) + strlen (server_name));
>> +       sprintf (socket_name, "%s/emacs%d/%s",
>> +                tmpdir, (int) geteuid (), server_name);

> There was neither much point in allocating an extra 100 bytes, nor
> in allocating 96 now. The formatted uid is not going to be 88
> characters long.

You really want to optimise this? ;-) If we assume that uid_t will
never have more than 64 bits, then a value of 28 should be sufficient
for the constant.

Ulrich (voting for 42)

Re: emacs client/server mode hardcodes /tmp path

[Juanma Barranquero]

On Wed, Oct 29, 2008 at 10:28, Ulrich Mueller <ulm@gentoo.org> wrote:

> You really want to optimise this? ;-)

It's a psychological optimization. Every time I see that 100 I look at
the code to see what I'm missing. 96 would be even worse, because it
does look meaningful.

> Ulrich (voting for 42)

42 is fine :-)

  Juanma

Re: emacs client/server mode hardcodes /tmp path

[Stefan Monnier]

>> You really want to optimise this? ;-)
> It's a psychological optimization. Every time I see that 100 I look at
> the code to see what I'm missing. 96 would be even worse, because it
> does look meaningful.
>> Ulrich (voting for 42)
> 42 is fine :-)

Please just leave it as 100.  The rest of the code uses 100 everywhere.
Otherwise, define a macro (which you can set to 42 if you really insist)
and use it everywhere.


        Stefan

Re: emacs client/server mode hardcodes /tmp path

[Juanma Barranquero]

On Wed, Oct 29, 2008 at 15:41, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Please just leave it as 100.  The rest of the code uses 100 everywhere.

Not in emacsclient.c. In src/, I see 100 mostly used as

  char tmp_buf[100];

and similar; but as a size for alloca (), xmalloc () etc. it appears
only a few times, alone or as part of an expression.

> Otherwise, define a macro (which you can set to 42 if you really insist)
> and use it everywhere.

I've already checked the patch with 32, which seems a fine default: a
power of two and small (and it is used a few times already on
emacsclient.c). 42 was obviously only a joke.

But if you insist I can change all these 32 to 100. Or use a macro for
all these EXTRA_SPACE, if you suggest a good name for it.

  Juanma

Re: emacs client/server mode hardcodes /tmp path

[Stefan Monnier]

>> Please just leave it as 100.  The rest of the code uses 100 everywhere.
> Not in emacsclient.c. In src/, I see 100 mostly used as

>   char tmp_buf[100];

> and similar; but as a size for alloca (), xmalloc () etc. it appears
> only a few times, alone or as part of an expression.

Whether it's for a static or dynamic allocation doesn't matter: the
places where some arbitrary number is used, it's usually 100.

>> Otherwise, define a macro (which you can set to 42 if you really insist)
>> and use it everywhere.

> I've already checked the patch with 32, which seems a fine default: a
> power of two and small (and it is used a few times already on
> emacsclient.c). 42 was obviously only a joke.

Actually 32 is no better than 42 or 100.  The important part is that
people shouldn't feel compelled to change 100 to 96 (or 32 to 28) when
they reduce a size by 4.

> But if you insist I can change all these 32 to 100. Or use a macro for
> all these EXTRA_SPACE, if you suggest a good name for it.

EXTRA_SPACE sounds fine,


        Stefan

Re: emacs client/server mode hardcodes /tmp path

[Juanma Barranquero]

On Wed, Oct 29, 2008 at 16:03, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Actually 32 is no better than 42 or 100.  The important part is that
> people shouldn't feel compelled to change 100 to 96 (or 32 to 28) when
> they reduce a size by 4.

That Ulrich felt the compulsion to change 100 to 96 shows how
intuitive is that 100 (or 32, of course).

> EXTRA_SPACE sounds fine,

OK.

  Juanma