]> ruderich.org/simon Gitweb - coloredstderr/coloredstderr.git/commitdiff
Overwrite tracked file descriptors when exporting COLORED_STDERR_FDS.
authorSimon Ruderich <simon@ruderich.org>
Fri, 21 Jun 2013 16:26:23 +0000 (18:26 +0200)
committerSimon Ruderich <simon@ruderich.org>
Fri, 21 Jun 2013 21:03:12 +0000 (23:03 +0200)
If the list of tracked file descriptors is modified to contain unwanted
values (e.g. when a parent process closes or redirects stderr), this
makes it possible to set the tracked file descriptors back to a given
list.

This problem occurred with startx which somehow closes or redirects
stderr. As COLORED_STDERR_FDS was set in the shell running startx, this
disabled coloring stderr for all processes started in the X11 session.

This is no longer an issue. As COLORED_STDERR_FDS is set in the shell
configuration file, it automatically sets the list to the correct value
when a shell is opened in the terminal emulator once X11 is running.

README
src/coloredstderr.c
src/constants.h
src/trackfds.h
tests/example_exec.c
tests/example_exec.expected
tests/lib.sh
tests/test_environment.sh

diff --git a/README b/README
index 967a6fd69a6c94d5c7be268c063b3ab418e1daad..79a40843429a4c66f346cf893be42c29f4834cb3 100644 (file)
--- a/README
+++ b/README
@@ -95,6 +95,10 @@ The following additional environment variables are available:
   terminal, e.g. when writing to a file. By default, only writes to a terminal
   are colored.
 
+All environment variables starting with 'COLORED_STDERR_PRIVATE_*' are
+internal variables used by the implementation and should not be set manually.
+See the source for details.
+
 
 To set custom colors as pre/post strings you can use the `$''` feature of Bash
 and Zsh:
index 00de8a588b080cee6b37d968f8641df954bd0544..586b0240d3df19b3a70a3f4a1d0692e81a36345f 100644 (file)
@@ -78,6 +78,9 @@ static size_t (*real_fwrite)(void const *, size_t, size_t, FILE *);
 static int initialized;
 /* Force hooked writes even when not writing to a tty. Used for tests. */
 static int force_write_to_non_tty;
+/* Was ENV_NAME_FDS found and used when init_from_environment() was called?
+ * This is not true if the process set it manually after initialization. */
+static int used_fds_set_by_user;
 
 
 #include "constants.h"
@@ -508,34 +511,23 @@ pid_t vfork(void) {
 
 /* Hook execve() and the other exec*() functions. Some shells use exec*() with
  * a custom environment which doesn't necessarily contain our updates to
- * ENV_NAME_FDS. It's also faster to update the environment only when
- * necessary, right before the exec() to pass it to the new process. */
+ * ENV_NAME_PRIVATE_FDS. It's also faster to update the environment only when
+ * necessary, right before the exec(), to pass it to the new program. */
 
 /* int execve(char const *, char * const [], char * const []) */
 HOOK_FUNC_DEF3(int, execve, char const *, filename, char * const *, argv, char * const *, env) {
     DLSYM_FUNCTION(real_execve, "execve");
 
-    int found = 0;
-    size_t index = 0;
-
-    /* Count arguments and search for existing ENV_NAME_FDS environment
-     * variable. */
+    /* Count environment variables. */
     size_t count = 0;
     char * const *x = env;
-    while (*x) {
-        if (!strncmp(*x, ENV_NAME_FDS "=", strlen(ENV_NAME_FDS) + 1)) {
-            found = 1;
-            index = count;
-        }
-
-        x++;
+    while (*x++) {
         count++;
     }
     /* Terminating NULL. */
     count++;
 
     char *env_copy[count + 1 /* space for our new entry if necessary */];
-    memcpy(env_copy, env, count * sizeof(char *));
 
     /* Make sure the information from the environment is loaded. We can't just
      * do nothing (like update_environment()) because the caller might pass a
@@ -544,17 +536,40 @@ HOOK_FUNC_DEF3(int, execve, char const *, filename, char * const *, argv, char *
         init_from_environment();
     }
 
-    char fds_env[strlen(ENV_NAME_FDS) + 1 + update_environment_buffer_size()];
-    strcpy(fds_env, ENV_NAME_FDS "=");
-    update_environment_buffer(fds_env + strlen(ENV_NAME_FDS) + 1);
+    char fds_env[strlen(ENV_NAME_PRIVATE_FDS)
+                 + 1 + update_environment_buffer_size()];
+    strcpy(fds_env, ENV_NAME_PRIVATE_FDS "=");
+    update_environment_buffer(fds_env + strlen(ENV_NAME_PRIVATE_FDS) + 1);
 
-    if (found) {
-        env_copy[index] = fds_env;
-    } else {
-        /* If the process removed ENV_NAME_FDS from the environment, re-add
-         * it. */
-        env_copy[count-1] = fds_env;
-        env_copy[count] = NULL;
+    int found = 0;
+    char **x_copy = env_copy;
+
+    /* Copy the environment manually; allows skipping elements. */
+    x = env;
+    while ((*x_copy = *x)) {
+        /* Remove ENV_NAME_FDS if we've already used its value. The new
+         * program must use the updated list from ENV_NAME_PRIVATE_FDS. */
+        if (used_fds_set_by_user
+                && !strncmp(*x, ENV_NAME_FDS "=", strlen(ENV_NAME_FDS) + 1)) {
+            x++;
+            continue;
+        /* Update ENV_NAME_PRIVATE_FDS. */
+        } else if (!strncmp(*x, ENV_NAME_PRIVATE_FDS "=",
+                            strlen(ENV_NAME_PRIVATE_FDS) + 1)) {
+            *x_copy = fds_env;
+            found = 1;
+        }
+
+        x++;
+        x_copy++;
+    }
+    /* The loop "condition" NULL-terminates env_copy. */
+
+    if (!found) {
+        /* If the process removed ENV_NAME_PRIVATE_FDS from the environment,
+         * re-add it. */
+        *x_copy++ = fds_env;
+        *x_copy++ = NULL;
     }
 
     return real_execve(filename, argv, env_copy);
index f195cc6e5fd775fb27334247c9f8deb8943fa20a..4cc69ca5e7a2931c2a574733288248240446dd76 100644 (file)
@@ -25,6 +25,7 @@
 #define ENV_NAME_PRE_STRING  "COLORED_STDERR_PRE"
 #define ENV_NAME_POST_STRING "COLORED_STDERR_POST"
 #define ENV_NAME_FORCE_WRITE "COLORED_STDERR_FORCE_WRITE"
+#define ENV_NAME_PRIVATE_FDS "COLORED_STDERR_PRIVATE_FDS"
 
 /* Strings written before/after each matched function. */
 #define DEFAULT_PRE_STRING  "\033[91m"
index f0ba8791d8485fd5441ae7bcd45109bdda482560..30693f47d1a07888bb578c665227f2ae3afc36d6 100644 (file)
@@ -72,8 +72,9 @@ static int init_tracked_fds_list(size_t count) {
 /* Load tracked file descriptors from the environment. The environment is used
  * to pass the information to child processes.
  *
- * ENV_NAME_FDS has the following format: Each descriptor as string followed
- * by a comma; there's a trailing comma. Example: "2,4,". */
+ * ENV_NAME_FDS and ENV_NAME_PRIVATE_FDS have the following format: Each
+ * descriptor as string followed by a comma; there's a trailing comma.
+ * Example: "2,4,". */
 static void init_from_environment(void) {
 #ifdef DEBUG
     debug("init_from_environment()\t\t[%d]\n", getpid());
@@ -92,14 +93,23 @@ static void init_from_environment(void) {
         force_write_to_non_tty = 1;
     }
 
+    /* Prefer user defined list of file descriptors, fall back to file
+     * descriptors passed through the environment from the parent process. */
     env = getenv(ENV_NAME_FDS);
+    if (env) {
+        used_fds_set_by_user = 1;
+    } else {
+        env = getenv(ENV_NAME_PRIVATE_FDS);
+    }
     if (!env) {
         errno = saved_errno;
         return;
     }
 #ifdef DEBUG
     debug("  getenv(\"%s\"): \"%s\"\n", ENV_NAME_FDS, env);
+    debug("  getenv(\"%s\"): \"%s\"\n", ENV_NAME_PRIVATE_FDS, env);
 #endif
+
     /* Environment must be treated read-only. */
     char env_copy[strlen(env) + 1];
     strcpy(env_copy, env);
@@ -225,16 +235,29 @@ static void update_environment(void) {
         return;
     }
 
+    int saved_errno = errno;
+
     char env[update_environment_buffer_size()];
     env[0] = 0;
 
     update_environment_buffer(env);
 
 #if 0
-    debug("    setenv(\"%s\", \"%s\", 1)\n", ENV_NAME_FDS, env);
+    debug("    setenv(\"%s\", \"%s\", 1)\n", ENV_NAME_PRIVATE_FDS, env);
 #endif
+    setenv(ENV_NAME_PRIVATE_FDS, env, 1 /* overwrite */);
 
-    setenv(ENV_NAME_FDS, env, 1 /* overwrite */);
+    /* Child processes must use ENV_NAME_PRIVATE_FDS to get the updated list
+     * of tracked file descriptors, not the static list provided by the user
+     * in ENV_NAME_FDS.
+     *
+     * But only remove it if the static list in ENV_NAME_FDS was loaded by
+     * init_from_environment() and merged into ENV_NAME_PRIVATE_FDS. */
+    if (used_fds_set_by_user) {
+        unsetenv(ENV_NAME_FDS);
+    }
+
+    errno = saved_errno;
 }
 
 
index bd5670079c9e270fb491028d8bbd086588c21402..2d22ce029fbcd56a267ca1a9d976d23dbfe5ae7e 100644 (file)
@@ -169,9 +169,38 @@ int main(int argc unused, char **argv) {
 
         execvp(argv[0], args);
         return EXIT_FAILURE;
-    } else {
+
+    /* Test handling of COLORED_STDERR_FDS. */
+
+    } else if (!skip--) {
+        /* And the rest. */
         close(3);
         close(8);
+
+        dup2(2, 5);
+
+        char *args[] = { argv0, NULL };
+        char *envp[] = { ldpreload, "COLORED_STDERR_FDS=5,", NULL };
+
+        execve(argv[0], args, envp);
+        return EXIT_FAILURE;
+    } else if (!skip--) {
+        char *args[] = { argv0, NULL };
+        char *envp[] = { ldpreload, NULL };
+
+        dup2(5, 6);
+        close(5);
+
+        execve(argv[0], args, envp);
+        return EXIT_FAILURE;
+    } else if (!skip--) {
+        close(6);
+
+        char *args[] = { argv0, NULL };
+        setenv("COLORED_STDERR_FDS", "2,", 1);
+
+        execv(argv[0], args);
+        return EXIT_FAILURE;
     }
 
 
@@ -292,13 +321,13 @@ int main(int argc unused, char **argv) {
     } else if (!skip--) {
         puts("argv[0] = |./example_exec|");
         puts("environ[0] = |TEST=54|");
-        puts("environ[2] = |COLORED_STDERR_FDS=2,|");
+        puts("environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,|");
         puts("");
         puts("argv[0] = |./example_exec|");
         puts("argv[1] = |foo|");
         puts("argv[2] = |bar|");
         puts("environ[0] = |TEST=55|");
-        puts("environ[2] = |COLORED_STDERR_FDS=2,|");
+        puts("environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,|");
         puts("");
 #endif
     }
index c79a3decdd27c65525f1d3393c0609f82fd970a8..61ca212e45c364701662b9682b44915eb562a676 100644 (file)
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 
 CHECKING COLORING.
 
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,3,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,|
 
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,3,4,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,4,|
 
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,3,4,5,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,4,5,|
 
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,3,4,5,6,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,4,5,6,|
 
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,3,4,7,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,4,7,|
 
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,3,8,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,8,|
+
+argv[0] = |./example_exec|
+environ[1] = |COLORED_STDERR_FDS=5,|
+environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,5,|
+
+argv[0] = |./example_exec|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=6,|
+
+argv[0] = |./example_exec|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=|
+environ[2] = |COLORED_STDERR_FDS=2,|
 
 
 CHECKING TRANSPARENCY.
 
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 environ[0] = |TEST=42|
-environ[2] = |COLORED_STDERR_FDS=2,|
+environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 argv[1] = |foo|
 argv[2] = |bar|
 environ[0] = |TEST=43|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 environ[0] = |TEST=44|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 argv[1] = |foo|
 argv[2] = |bar|
 environ[0] = |TEST=45|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 environ[0] = |TEST=46|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 argv[1] = |foo|
 argv[2] = |bar|
 environ[0] = |TEST=47|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
-environ[1] = |COLORED_STDERR_FDS=2,|
+environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 environ[0] = |TEST=48|
-environ[2] = |COLORED_STDERR_FDS=2,|
+environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 argv[1] = |foo|
 argv[2] = |bar|
 environ[0] = |TEST=49|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 environ[0] = |TEST=50|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 argv[1] = |foo|
 argv[2] = |bar|
 environ[0] = |TEST=51|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 environ[0] = |TEST=52|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 argv[1] = |foo|
 argv[2] = |bar|
 environ[0] = |TEST=53|
 environ[1] = |FOO=|
-environ[3] = |COLORED_STDERR_FDS=2,|
+environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 environ[0] = |TEST=54|
-environ[2] = |COLORED_STDERR_FDS=2,|
+environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 argv[0] = |./example_exec|
 argv[1] = |foo|
 argv[2] = |bar|
 environ[0] = |TEST=55|
-environ[2] = |COLORED_STDERR_FDS=2,|
+environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,|
 
 Done.
index 3427b5608fc36cdf7dca0824fbca4b4f97979f73..2f2c736711fc787fb74ca066aac4d3afb68b4cc6 100644 (file)
@@ -40,7 +40,10 @@ fi
 LC_ALL=C
 unset LANGUAGE
 
-# Set default COLORED_STDERR_FDS value.
+# Clear user defined variables.
+unset COLORED_STDERR_FDS
+unset COLORED_STDERR_FORCE_WRITE
+# Set default COLORED_STDERR_PRIVATE_FDS value.
 fds=2,
 
 
@@ -78,9 +81,9 @@ run_test() {
     (
         # Standard setup.
         LD_PRELOAD="$library"
-        COLORED_STDERR_FDS="$fds"
+        COLORED_STDERR_PRIVATE_FDS="$fds"
         export LD_PRELOAD
-        export COLORED_STDERR_FDS
+        export COLORED_STDERR_PRIVATE_FDS
 
         # Change pre/post strings for simpler testing.
         COLORED_STDERR_PRE='>STDERR>'
index a530718735dd20358569192282ecf4691a3f22c1..47a02a25f71960e369aed28949102f9e1be3ae94 100755 (executable)
@@ -18,7 +18,7 @@
 test "x$srcdir" = x && srcdir=.
 . "$srcdir/lib.sh"
 
-# Test unexpected values for COLORED_STDERR_FDS environment variable.
+# Test unexpected values for COLORED_STDERR_PRIVATE_FDS environment variable.
 
 # Empty fields.
 fds=
@@ -44,3 +44,18 @@ test_program_subshell example example_environment
 fds=-20,-30,2,-1,
 test_program          example example_environment
 test_program_subshell example example_environment
+
+# Test COLORED_STDERR_FDS overwrites COLORED_STDERR_PRIVATE_FDS. Additional
+# tests in example_exec.
+
+fds=
+COLORED_STDERR_FDS=2,
+export COLORED_STDERR_FDS
+test_program          example example_environment
+test_program_subshell example example_environment
+
+fds=2,
+COLORED_STDERR_FDS=
+export COLORED_STDERR_FDS
+test_program          example example_environment_empty
+test_program_subshell example example_environment_empty