[ruby-core:35491] Re: /proc/$PID/environ in Linux

KOSAKI Motohiro <[e-mail]> wrote:
> > Can we do this lazily and only as a last resort when argv doesn't have
> > space?  I can look into supplying patches this weekend, at least for
> > the Linux use case.
> 
> This is OK. I'll commit it if you will post it.

OK thanks, do you prefer attached or inline? I figured inline since you
do Linux :)

I also have a question for nobu below, too:

From 790a2f043035c9a38222db3146e866ea0b2e3db7 Mon Sep 17 00:00:00 2001
From: Eric Wong <[e-mail]>
Date: Sun, 13 Mar 2011 08:39:15 +0000
Subject: [PATCH] ruby.c: defer moving environ for $0 changes

This allows /proc/<pid>/environ on Linux to remain useful
for minor $0 modifications.  This is especially useful for
Ruby because argv storage space is inflated from shebang
usage so there's less need for environ space.

I've only tested this on Linux 2.6

While preparing this change, I went through history and
was confused by one hunk in r27468:

	------------------------------------------------------------------------
	r27468 | nobu | 2010-04-24 04:06:59 +0000 (Sat, 24 Apr 2010) | 8 lines

	* ruby.c (set_arg0): keep empty strings.

	--- a/ruby.c
	+++ b/ruby.c
	@@ -1712,8 +1725,8 @@ set_arg0(VALUE val, ID id)
	     setproctitle("%.*s", (int)i, s);
	 #else

	-    if ((size_t)i >= origarg.len) {
	-	i = (long)(origarg.len - 1);
	+    if ((size_t)i > origarg.len - origarg.argc) {
	+	i = (long)(origarg.len - origarg.argc);
	     }

	     memcpy(origarg.argv[0], s, i);

All the other hunks in r27468 make sense except the above.  I
followed it for the new check I added in this commit, but I
don't understand how origarg.argc factors into limiting the
length of argv[0].

From what I can tell, argv[j...argc] below do not need
additional storage, they just point at 't' which never
changes...

	@@ -1724,7 +1737,7 @@ set_arg0(VALUE val, ID id)
		*t = '\0';

		if ((size_t)(i + 1) < origarg.len) {
	-	    memset(t + 1, ' ', origarg.len - i - 1);
	+	    memset(t + 1, '\0', origarg.len - i - 1);
		}
		for (j = 1; j < origarg.argc; j++) {
		    origarg.argv[j] = t;

It's late, so maybe I'm missing something obvious.  I don't trust
my arithmetic skills much.
---
 ruby.c |   46 +++++++++++++++++++++++++++++++++-------------
 1 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/ruby.c b/ruby.c
index c5d46ed..1b2bd7f 100644
--- a/ruby.c
+++ b/ruby.c
@@ -1733,6 +1733,28 @@ rb_load_file(const char *fname)
 
 #ifdef USE_ENVSPACE_FOR_ARG0
 extern char **environ;
+static size_t envspace_capa;
+static void move_environ(void)
+{
+# if defined(HAVE_SETENV) && defined(HAVE_UNSETENV)
+    char *t;
+    int i;
+    size_t len = 0;
+
+    for (i = 0; environ[i]; i++)
+	len += strlen(environ[i]) + 1;
+
+    t = xmalloc(len);
+    for (i = 0; environ[i]; i++) {
+	len = strlen(environ[i]) + 1;
+	memcpy(t, environ[i], len);
+	environ[i] = t;
+	t += len;
+    }
+# else
+    ruby_setenv("", NULL); /* duplicate environ vars */
+# endif
+}
 #endif
 
 static size_t
@@ -1755,6 +1777,7 @@ get_arglen(int argc, char **argv)
     }
 #if defined(USE_ENVSPACE_FOR_ARG0)
     if (environ && (s+1 == environ[0])) {
+	char *pre_env = s;
 	s++;
 	s += strlen(s);
 	for (i = 1; environ[i]; i++) {
@@ -1763,19 +1786,8 @@ get_arglen(int argc, char **argv)
 		s += strlen(s);	/* this one is ok too */
 	    }
 	}
-# if defined(HAVE_SETENV) && defined(HAVE_UNSETENV)
-	{
-	    char *t = malloc(s - environ[0] + 1);
-	    for (i = 0; environ[i]; i++) {
-		size_t len = strlen(environ[i]) + 1;
-		memcpy(t, environ[i], len);
-		environ[i] = t;
-		t += len;
-	    }
-	}
-# else
-	ruby_setenv("", NULL); /* duplicate environ vars */
-# endif
+	envspace_capa = s - pre_env;
+	s = pre_env;
     }
 #endif
     return s - argv[0];
@@ -1810,6 +1822,14 @@ set_arg0(VALUE val, ID id)
     setproctitle("%.*s", (int)i, s);
 #else
 
+# ifdef USE_ENVSPACE_FOR_ARG0
+    if (envspace_capa > 0 && (size_t)i > origarg.len - origarg.argc) {
+	move_environ();
+	origarg.len += envspace_capa;
+	envspace_capa = 0;
+    }
+# endif
+
     if ((size_t)i > origarg.len - origarg.argc) {
 	i = (long)(origarg.len - origarg.argc);
     }
-- 
Eric Wong