This is the mail archive of the
java-patches@gcc.gnu.org
mailing list for the Java project.
Re: Patch: PR libgcj/23758, Fix theoretical race PosixProcess.
- From: David Daney <ddaney at avtrex dot com>
- To: tromey at redhat dot com
- Cc: GCJ-patches <java-patches at gcc dot gnu dot org>
- Date: Mon, 09 Apr 2007 10:46:38 -0700
- Subject: Re: Patch: PR libgcj/23758, Fix theoretical race PosixProcess.
- References: <4619D39D.9020706@avtrex.com> <m3r6qtmr9q.fsf@localhost.localdomain>
Tom Tromey wrote:
"David" == David Daney <ddaney@avtrex.com> writes:
David> This patch moves the mallocs out of the danger zone in
David> natPosixProcess.cc
David> OK to commit?
Two nits -- fix these and it is ok.
David> + // explicitly. We need three extra slots. Potentailly PATH
Typo, should be "potentially".
David> + if (0 == strncmp (env[i], "PATH=", sizeof("PATH=")))
David> + need_path = false;
David> + if (0 == strncmp (env[i], "LD_LIBRARY_PATH=",
David> + sizeof("LD_LIBRARY_PATH=")))
David> + need_ld_library_path = false;
Use "! strncmp" instead. That's libgcj convention.
This is the version I committed:
2007-04-09 David Daney <ddaney@avtrex.com>
PR libgcj/23758
* java/lang/natPosixProcess.cc (nativeSpawn): Move building of
environment before the fork.
* testsuite/libjava.lang/Process_7.java: New test.
* testsuite/libjava.lang/Process_7.out: Its expected results.
* testsuite/libjava.lang/Process_7.jar: Generated file.
Index: testsuite/libjava.lang/Process_7.out
===================================================================
--- testsuite/libjava.lang/Process_7.out (revision 0)
+++ testsuite/libjava.lang/Process_7.out (revision 123676)
@@ -0,0 +1 @@
+ok
Index: testsuite/libjava.lang/Process_7.java
===================================================================
--- testsuite/libjava.lang/Process_7.java (revision 0)
+++ testsuite/libjava.lang/Process_7.java (revision 123676)
@@ -0,0 +1,45 @@
+// Verify we can modify the environment.
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.util.Map;
+
+
+
+public class Process_7
+{
+ public static void main(String[] args)
+ {
+ try
+ {
+ ProcessBuilder pb = new ProcessBuilder("env");
+ Map<String, String> e = pb.environment();
+ e.clear();
+ String v = "process7_value";
+ String k = "PROCESS_7_KEY";
+ e.put(k, v);
+ Process p = pb.start();
+ InputStream is = p.getInputStream();
+ InputStreamReader isr = new InputStreamReader(is);
+ BufferedReader br = new BufferedReader(isr);
+ boolean found = false;
+
+ String result;
+ while ((result = br.readLine()) != null)
+ {
+ if (result.equals(k + '=' + v))
+ found = true;
+ }
+ if (!found)
+ {
+ System.out.println("bad");
+ return;
+ }
+ System.out.println("ok");
+ }
+ catch (Exception ex)
+ {
+ System.out.println(ex.toString());
+ }
+ }
+}
Index: java/lang/natPosixProcess.cc
===================================================================
--- java/lang/natPosixProcess.cc (revision 123675)
+++ java/lang/natPosixProcess.cc (revision 123676)
@@ -1,6 +1,7 @@
// natPosixProcess.cc - Native side of POSIX process code.
-/* Copyright (C) 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006, 2007 Free Software Foundation
+/* Copyright (C) 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006, 2007
+ Free Software Foundation
This file is part of libgcj.
@@ -248,16 +249,57 @@ java::lang::PosixProcess::nativeSpawn ()
if (envp)
{
- env = (char **) _Jv_Malloc ((envp->length + 1) * sizeof (char *));
+ bool need_path = true;
+ bool need_ld_library_path = true;
+ int i;
+
+ // Preserve PATH and LD_LIBRARY_PATH unless specified
+ // explicitly. We need three extra slots. Potentially PATH
+ // and LD_LIBRARY_PATH will be added plus the NULL
+ // termination.
+ env = (char **) _Jv_Malloc ((envp->length + 3) * sizeof (char *));
elts = elements (envp);
// Initialize so we can gracefully recover.
- for (int i = 0; i <= envp->length; ++i)
+ for (i = 0; i < envp->length + 3; ++i)
env[i] = NULL;
- for (int i = 0; i < envp->length; ++i)
- env[i] = new_string (elts[i]);
- env[envp->length] = NULL;
+ for (i = 0; i < envp->length; ++i)
+ {
+ env[i] = new_string (elts[i]);
+ if (!strncmp (env[i], "PATH=", sizeof("PATH=")))
+ need_path = false;
+ if (!strncmp (env[i], "LD_LIBRARY_PATH=",
+ sizeof("LD_LIBRARY_PATH=")))
+ need_ld_library_path = false;
+ }
+
+ if (need_path)
+ {
+ char *path_val = getenv ("PATH");
+ if (path_val)
+ {
+ env[i] = (char *) _Jv_Malloc (strlen (path_val) +
+ sizeof("PATH=") + 1);
+ strcpy (env[i], "PATH=");
+ strcat (env[i], path_val);
+ i++;
+ }
+ }
+ if (need_ld_library_path)
+ {
+ char *path_val = getenv ("LD_LIBRARY_PATH");
+ if (path_val)
+ {
+ env[i] =
+ (char *) _Jv_Malloc (strlen (path_val) +
+ sizeof("LD_LIBRARY_PATH=") + 1);
+ strcpy (env[i], "LD_LIBRARY_PATH=");
+ strcat (env[i], path_val);
+ i++;
+ }
+ }
+ env[i] = NULL;
}
// We allocate this here because we can't call malloc() after
@@ -303,29 +345,7 @@ java::lang::PosixProcess::nativeSpawn ()
{
// Child process, so remap descriptors, chdir and exec.
if (envp)
- {
- // Preserve PATH and LD_LIBRARY_PATH unless specified
- // explicitly.
- char *path_val = getenv ("PATH");
- char *ld_path_val = getenv ("LD_LIBRARY_PATH");
- environ = env;
- if (path_val && getenv ("PATH") == NULL)
- {
- char *path_env =
- (char *) _Jv_Malloc (strlen (path_val) + 5 + 1);
- strcpy (path_env, "PATH=");
- strcat (path_env, path_val);
- putenv (path_env);
- }
- if (ld_path_val && getenv ("LD_LIBRARY_PATH") == NULL)
- {
- char *ld_path_env =
- (char *) _Jv_Malloc (strlen (ld_path_val) + 16 + 1);
- strcpy (ld_path_env, "LD_LIBRARY_PATH=");
- strcat (ld_path_env, ld_path_val);
- putenv (ld_path_env);
- }
- }
+ environ = env;
// We ignore errors from dup2 because they should never occur.
dup2 (outp[0], 0);
@@ -344,7 +364,7 @@ java::lang::PosixProcess::nativeSpawn ()
close (outp[0]);
close (outp[1]);
close (msgp[0]);
-
+
// Change directory.
if (path != NULL)
{