public inbox for frysk-cvs@sourceware.org
help / color / mirror / Atom feed
* [SCM]  master: Change frysk.sys.Fork's environ param to a String[]; fixes mem leaks.
@ 2008-06-02  1:52 cagney
  0 siblings, 0 replies; only message in thread
From: cagney @ 2008-06-02  1:52 UTC (permalink / raw)
  To: frysk-cvs

The branch, master has been updated
       via  c6ead450f9b66216ef65acc2822af57d8a268cc8 (commit)
      from  0aeb3adfa7c2316dcf663be037bc1338c1321653 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email.

- Log -----------------------------------------------------------------
commit c6ead450f9b66216ef65acc2822af57d8a268cc8
Author: Andrew Cagney <cagney@redhat.com>
Date:   Sun Jun 1 21:48:19 2008 -0400

    Change frysk.sys.Fork's environ param to a String[]; fixes mem leaks.
    
    The frysk.sys.Environ code was creating an environ[] vector and
    returning it as a jlong; and then it was passed to the fork code.  It
    was never freed.
    
    Also, the code setting libraries would smash any existing
    LD_LIBRARY_PATH entries, the additional libraries are now appended.
    
    frysk-core/frysk/proc/ChangeLog
    2008-06-01  Andrew Cagney  <cagney@redhat.com>
    
    	* TestEnviron.java: New.
    	* Environ.java: New.
    
    frysk-sys/frysk/config/ChangeLog
    2008-06-01  Andrew Cagney  <cagney@redhat.com>
    
    	* Host.java (static): Load the runtime.
    
    frysk-sys/frysk/sys/ChangeLog
    2008-06-01  Andrew Cagney  <cagney@redhat.com>
    
    	* Fork.java (spawn, ptrace, daemon, utrace): Change exe
    	parameter's type to String; change environ parameter's type to
    	String[]; replace lib with environ.
    	* jni/Fork.cxx: Update.
    	* cni/Fork.cxx: Update.
    	* cni/Fork.hxx: Update.
    	* jni/Fork.hxx: Update.
    	* Environ.java: Rewrite.
    	* cni/Environ.cxx (Environ::getenv): Update. New.
    	* jni/Environ.cxx (Environ::getenv): Update. New.
    	* TestEnviron.java: Re-implement.

-----------------------------------------------------------------------

Summary of changes:
 frysk-core/frysk/proc/ChangeLog                    |    5 +
 .../sys => frysk-core/frysk/proc}/Environ.java     |   99 ++++++++++----------
 .../sys => frysk-core/frysk/proc}/TestEnviron.java |   67 ++++++++------
 frysk-core/frysk/proc/live/LinuxPtraceHost.java    |   16 +++-
 frysk-sys/frysk/config/ChangeLog                   |    4 +
 frysk-sys/frysk/config/Host.java                   |    4 +
 frysk-sys/frysk/sys/ChangeLog                      |   14 +++
 frysk-sys/frysk/sys/Environ.java                   |   86 +----------------
 frysk-sys/frysk/sys/Fork.java                      |   66 +++++++------
 frysk-sys/frysk/sys/TestEnviron.java               |   32 ++-----
 frysk-sys/frysk/sys/cni/Environ.cxx                |   49 +++-------
 frysk-sys/frysk/sys/cni/Fork.cxx                   |   23 ++---
 frysk-sys/frysk/sys/cni/Fork.hxx                   |    9 ++-
 frysk-sys/frysk/sys/jni/Environ.cxx                |   30 ++----
 frysk-sys/frysk/sys/jni/Fork.cxx                   |   27 +++--
 frysk-sys/frysk/sys/jni/Fork.hxx                   |   14 ++-
 frysk-sys/frysk/sys/jni/PipePair.cxx               |    3 +-
 frysk-sys/frysk/sys/jni/PseudoTerminal.cxx         |    6 +-
 18 files changed, 254 insertions(+), 300 deletions(-)
 copy {frysk-sys/frysk/sys => frysk-core/frysk/proc}/Environ.java (62%)
 copy {frysk-sys/frysk/sys => frysk-core/frysk/proc}/TestEnviron.java (60%)

First 500 lines of diff:
diff --git a/frysk-core/frysk/proc/ChangeLog b/frysk-core/frysk/proc/ChangeLog
index 47e0c51..99716be 100644
--- a/frysk-core/frysk/proc/ChangeLog
+++ b/frysk-core/frysk/proc/ChangeLog
@@ -1,3 +1,8 @@
+2008-06-01  Andrew Cagney  <cagney@redhat.com>
+
+	* TestEnviron.java: New.
+	* Environ.java: New.
+
 2008-05-22  Andrew Cagney  <cagney@redhat.com>
 
 	* TestBreakpoints.java: Update, use PipePair.daemon.
diff --git a/frysk-sys/frysk/sys/Environ.java b/frysk-core/frysk/proc/Environ.java
similarity index 62%
copy from frysk-sys/frysk/sys/Environ.java
copy to frysk-core/frysk/proc/Environ.java
index ff4be00..dd8dd0d 100644
--- a/frysk-sys/frysk/sys/Environ.java
+++ b/frysk-core/frysk/proc/Environ.java
@@ -37,92 +37,93 @@
 // version and license this file solely under the GPL without
 // exception.
 
-package frysk.sys;
+package frysk.proc;
 
-import java.util.Collection;
 import java.util.HashMap;
+import java.util.Map.Entry;
 import java.util.Iterator;
 import java.util.Set;
 
 /**
- * Interface to the process environment.
+ * The environment vector.
  */
-public class Environ
-{
-    private HashMap env;
+public class Environ {
+    private final HashMap environ;
 
-    public Environ ()
-    {
-	env = new HashMap();   
-	getEnvironment();
+    /**
+     * Create a new empty environment.
+     */
+    public Environ() {
+	environ = new HashMap();   
     }
-    
     /**
-     * Construct an environment hash given an existing environ.  Used for testing
+     * Create a new environment populated by the existing environ.
      */
-    Environ (long environ)
-    {
-	env = new HashMap();   
-	getEnvironment(environ);
+    public Environ(String[] environ) {
+	this();
+	put(environ);
     }
-    
+
     /**
-     * Get the environment hash.  Used for testing
-     * @return the array of environment values.
+     * Return the environ as a string array.
      */
-    HashMap getEnvHash() {
+    public String[] toStringArray() {
+        Set entries = environ.entrySet();
+	String[] env = new String[environ.size()];
+	int j = 0;
+	for (Iterator i = entries.iterator(); i.hasNext(); ) {
+	    Entry e = (Entry)i.next();
+            String name = (String)e.getKey();
+	    String value = (String)e.getValue();
+	    env[j++] = name + "=" + value;
+	}
 	return env;
     }
-    
+
     /**
      * Get an environment variable.
      * @param name is the environment variable name.
      * @return the value of the variable.
      */
-    String getEnv(String name) {
-	return (String)env.get(name);
+    public String get(String name) {
+	return (String)environ.get(name);
     }
     
     /**
-     * Set the value of an environment variable.
+     * Put the variable into the environ set with the provided value.
      * @param name is the environment variable name.
      * @param value is the environment variable value.
      */
-    void setEnv(String name, String value) {
-	env.put(name, value);
+    public void put(String name, String value) {
+	environ.put(name, value);
     }
 
     /**
-     * Used by CNI code to add an environment variable.
+     * Decode then add an environment variable.
      * @param name is the variable=value pair.
      */
-    void addEnviron(String name) {
-	if (name.length() != 0) {
-	    String envMember [] = name.split("=");
-	    env.put(envMember[0], envMember.length == 2 ? envMember[1] : "");
+    public void put(String name) {
+	String[] member = name.split("=");
+	if (member.length == 2) {
+	    environ.put(member[0], member[1]);
+	} else {
+	    environ.put(member[0], "");
 	}
     }
 
     /**
-     * Put the environment variables in env into char **environ form.
-     * @return the environ.
+     * Put all elements of the the ENVIRON array into the ENVIRON set.
      */
-    long putEnviron() {
-        Set keys = env.keySet();
-        Collection values= env.values();
-        Iterator valueIter = values.iterator();
-        Iterator keyIter= keys.iterator();
-        int idx = 0;
-        String[] envs = new String[values.size()];
-
-        while (keyIter.hasNext()) {
-            envs[idx] = (String)keyIter.next() + "=" + valueIter.next();
-            idx += 1;
-        }
-        return putEnvironment(envs);
+    public void put(String[] environ) {
+	for (int i = 0; i < environ.length; i++) {
+	    put(environ[i]);
+	}
     }
 
-    native void getEnvironment();
-    native void getEnvironment(long environ);
-    native long putEnvironment(String[] envs);
+    /**
+     * Delete the entry.
+     */
+    public void remove(String name) {
+	environ.remove(name);
+    }
 }
diff --git a/frysk-sys/frysk/sys/TestEnviron.java b/frysk-core/frysk/proc/TestEnviron.java
similarity index 60%
copy from frysk-sys/frysk/sys/TestEnviron.java
copy to frysk-core/frysk/proc/TestEnviron.java
index 4a5e050..38b026c 100644
--- a/frysk-sys/frysk/sys/TestEnviron.java
+++ b/frysk-core/frysk/proc/TestEnviron.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2008, Red Hat Inc.
+// Copyright 2008 Red Hat Inc.
 //
 // FRYSK is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by
@@ -37,36 +37,49 @@
 // version and license this file solely under the GPL without
 // exception.
 
-package frysk.sys;
+package frysk.proc;
 
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Set;
-
-import frysk.junit.TestCase;
+import java.util.Arrays;
+import frysk.testbed.TestLib;
+import frysk.rsl.Log;
 
 /**
- * Test environ support
+ * Test an Environment vector.
  */
-public class TestEnviron extends TestCase {
+public class TestEnviron extends TestLib {
+    private final Log fine = Log.fine(TestEnviron.class);
+
+    private void check(String test, String[] value, Environ environ) {
+	String[] strings = environ.toStringArray();
+	fine.log("check", value, "environ", environ, "strings", strings);
+	Arrays.sort(strings);
+	assertEquals(test, value, strings);
+    }
 
-    public void testEnviron() {
-        Environ env1 = new Environ();
-        // Grab the environment hash table
-	HashMap env1Hash = env1.getEnvHash();
-	Set env1Keys = env1Hash.keySet();
-	// Load the environment from the environment hash table
-	long environ = env1.putEnviron();
-	// Create a new environment hash table from that environment
-	Environ env2 = new Environ(environ);
-	HashMap env2Hash = env2.getEnvHash();
-	
-	// Ensure that both environment hash tables match
-        Iterator iterator = env1Keys.iterator();
-        while(iterator.hasNext()){
-            String key = (String) iterator.next();
-            assertTrue ("env value test fails for key=" + key, 
-        	    ((String)env1Hash.get(key)).compareTo((String)env2Hash.get(key)) == 0);
-        }
+    public void testEmpty() {
+	Environ environ = new Environ();
+	check("empty environ", new String[0], environ);
+    }
+    public void testFull() {
+	String[] strings = new String[] { "A=a", "B=b" };
+	Environ environ = new Environ(strings);
+	check("full environ", strings, environ);
+    }
+    public void testGet() {
+	String[] strings = new String[] { "A=a", "B=b" };
+	Environ environ = new Environ(strings);
+	assertEquals("get", "b", environ.get("B"));
+    }
+    public void testPut() {
+	String[] strings = new String[] { "A=a", "B=b" };
+	Environ environ = new Environ(strings);
+	environ.put("B", "bb");
+	check("put", new String[] { "A=a", "B=bb" }, environ);
+    }
+    public void testRemove() {
+	String[] strings = new String[] { "A=a", "B=b" };
+	Environ environ = new Environ(strings);
+	environ.remove("B");
+	check("remove", new String[] { "A=a" }, environ);
     }
 }
diff --git a/frysk-core/frysk/proc/live/LinuxPtraceHost.java b/frysk-core/frysk/proc/live/LinuxPtraceHost.java
index d5bdb52..1c10aab 100644
--- a/frysk-core/frysk/proc/live/LinuxPtraceHost.java
+++ b/frysk-core/frysk/proc/live/LinuxPtraceHost.java
@@ -39,6 +39,7 @@
 
 package frysk.proc.live;
 
+import frysk.proc.Environ;
 import java.io.File;
 import java.util.HashSet;
 import frysk.event.EventLoop;
@@ -270,8 +271,21 @@ public class LinuxPtraceHost extends LiveHost {
 		    fine.log(LinuxPtraceHost.this,
 			     "execute - requestCreateAttachedProc",
 			     exe);
+		    // If there are libs, construct an environ.
+		    Environ environ = null;
+		    if (libs != null) {
+			environ = new Environ(frysk.sys.Environ.get());
+			String ldLibraryPath = environ.get("LD_LIBRARY_PATH");
+			if (ldLibraryPath != null) {
+			    environ.put("LD_LIBRARY_PATH",
+					libs + ":" + ldLibraryPath);
+			} else {
+			    environ.put("LD_LIBRARY_PATH", libs);
+			}
+		    }
 		    ProcessIdentifier pid
-			= Fork.ptrace(exe, stdin, stdout, stderr, args, libs);
+			= Fork.ptrace(exe, stdin, stdout, stderr,
+				      args, environ.toStringArray());
 		    // See if the Host knows about this task.
 		    ProcessIdentifier myTid = Tid.get();
 		    LinuxPtraceTask myTask = getTask(myTid);
diff --git a/frysk-sys/frysk/config/ChangeLog b/frysk-sys/frysk/config/ChangeLog
index 899d178..59ac944 100644
--- a/frysk-sys/frysk/config/ChangeLog
+++ b/frysk-sys/frysk/config/ChangeLog
@@ -1,3 +1,7 @@
+2008-06-01  Andrew Cagney  <cagney@redhat.com>
+
+	* Host.java (static): Load the runtime.
+
 2008-05-29  Andrew Cagney  <cagney@redhat.com>
 
 	* Runtime.java (nativeCall): New.
diff --git a/frysk-sys/frysk/config/Host.java b/frysk-sys/frysk/config/Host.java
index ce0f644..6df656b 100644
--- a/frysk-sys/frysk/config/Host.java
+++ b/frysk-sys/frysk/config/Host.java
@@ -44,6 +44,10 @@ package frysk.config;
  */
 
 public class Host {
+    static {
+	Runtime.load();
+    }
+
     /**
      * The word size of the host architecture (this is the
      * architecture for which the frysk executable built).
diff --git a/frysk-sys/frysk/sys/ChangeLog b/frysk-sys/frysk/sys/ChangeLog
index d26ad79..48548cb 100644
--- a/frysk-sys/frysk/sys/ChangeLog
+++ b/frysk-sys/frysk/sys/ChangeLog
@@ -1,3 +1,17 @@
+2008-06-01  Andrew Cagney  <cagney@redhat.com>
+
+	* Fork.java (spawn, ptrace, daemon, utrace): Change exe
+	parameter's type to String; change environ parameter's type to
+	String[]; replace lib with environ.
+	* jni/Fork.cxx: Update.
+	* cni/Fork.cxx: Update.
+	* cni/Fork.hxx: Update.
+	* jni/Fork.hxx: Update.
+	* Environ.java: Rewrite.
+	* cni/Environ.cxx (Environ::getenv): Update. New.
+	* jni/Environ.cxx (Environ::getenv): Update. New.
+	* TestEnviron.java: Re-implement.
+
 2008-05-25  Andrew Cagney  <cagney@redhat.com>
 
 	* jni/FileDescriptor.cxx: Use jbyteArrayElements / jstringUTFChars.
diff --git a/frysk-sys/frysk/sys/Environ.java b/frysk-sys/frysk/sys/Environ.java
index ff4be00..6400484 100644
--- a/frysk-sys/frysk/sys/Environ.java
+++ b/frysk-sys/frysk/sys/Environ.java
@@ -39,90 +39,12 @@
 
 package frysk.sys;
 
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Set;
-
 /**
  * Interface to the process environment.
  */
-public class Environ
-{
-    private HashMap env;
-
-    public Environ ()
-    {
-	env = new HashMap();   
-	getEnvironment();
-    }
-    
-    /**
-     * Construct an environment hash given an existing environ.  Used for testing
-     */
-    Environ (long environ)
-    {
-	env = new HashMap();   
-	getEnvironment(environ);
-    }
-    
-    /**
-     * Get the environment hash.  Used for testing
-     * @return the array of environment values.
-     */
-    HashMap getEnvHash() {
-	return env;
-    }
-    
-    /**
-     * Get an environment variable.
-     * @param name is the environment variable name.
-     * @return the value of the variable.
-     */
-    String getEnv(String name) {
-	return (String)env.get(name);
-    }
-    
-    /**
-     * Set the value of an environment variable.
-     * @param name is the environment variable name.
-     * @param value is the environment variable value.
-     */
-    void setEnv(String name, String value) {
-	env.put(name, value);
+public class Environ {
+    public static String[] get() {
+	return getenv();
     }
-
-    /**
-     * Used by CNI code to add an environment variable.
-     * @param name is the variable=value pair.
-     */
-    void addEnviron(String name) {
-	if (name.length() != 0) {
-	    String envMember [] = name.split("=");
-	    env.put(envMember[0], envMember.length == 2 ? envMember[1] : "");
-	}
-    }
-
-    /**
-     * Put the environment variables in env into char **environ form.
-     * @return the environ.
-     */
-    long putEnviron() {
-        Set keys = env.keySet();
-        Collection values= env.values();
-        Iterator valueIter = values.iterator();
-        Iterator keyIter= keys.iterator();
-        int idx = 0;
-        String[] envs = new String[values.size()];
-
-        while (keyIter.hasNext()) {
-            envs[idx] = (String)keyIter.next() + "=" + valueIter.next();
-            idx += 1;
-        }
-        return putEnvironment(envs);
-    }
-
-    native void getEnvironment();
-    native void getEnvironment(long environ);
-    native long putEnvironment(String[] envs);
+    private static native String[] getenv();
 }
diff --git a/frysk-sys/frysk/sys/Fork.java b/frysk-sys/frysk/sys/Fork.java
index 99dc9a2..7e2f39c 100644
--- a/frysk-sys/frysk/sys/Fork.java
+++ b/frysk-sys/frysk/sys/Fork.java
@@ -47,18 +47,18 @@ import java.io.File;
  */
 
 public final class Fork {
-    private static native int spawn(File exe,
+    private static native int spawn(String exe,
 				    String in, String out, String err,
-				    String[] args, long environ);
-    private static native int ptrace(File exe,
+				    String[] args, String[] environ);
+    private static native int ptrace(String exe,
 				     String in, String out, String err,
-				     String[] args, long environ);
-    private static native int utrace(File exe,
+				     String[] args, String[] environ);
+    private static native int utrace(String exe,
 				     String in, String out, String err,
-				     String[] args, long environ);
-    private static native int daemon(File exe,
+				     String[] args, String[] environ);
+    private static native int daemon(String exe,
 				     String in, String out, String err,
-				     String[] args, long environ);
+				     String[] args, String[] environ);
 
     /**
      * Create a child process running EXE with arguments ARGS[0..].
@@ -68,8 +68,9 @@ public final class Fork {
     public static ProcessIdentifier exec(File exe,
 					 String in, String out,
 					 String err, String[] args) {
-	return ProcessIdentifierFactory.create
-	    (spawn(exe, in, out, err, args, 0));
+	return ProcessIdentifierFactory.create(spawn(exe.getPath(),
+						     in, out, err,
+						     args, null));
     }
     /**
      * Create a child process running EXE with arguments ARGS[0..].
@@ -78,16 +79,18 @@ public final class Fork {
      */
     public static ProcessIdentifier exec(String in, String out,
 					 String err, String[] args) {
-	return ProcessIdentifierFactory.create
-	    (spawn(new File(args[0]), in, out, err, args, 0));
+	return ProcessIdentifierFactory.create(spawn(args[0],
+						     in, out, err,
+						     args, null));
     }
     /**
      * Create a child process running ARGS[0] with arguments
      * ARGS[0..].
      */
     public static ProcessIdentifier exec(String[] args) {
-	return ProcessIdentifierFactory.create
-	    (spawn(new File(args[0]), null, null, null, args, 0));
+	return ProcessIdentifierFactory.create(spawn(args[0],
+						     null, null, null,
+						     args, null));


hooks/post-receive
--
frysk system monitor/debugger


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2008-06-02  1:52 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-02  1:52 [SCM] master: Change frysk.sys.Fork's environ param to a String[]; fixes mem leaks cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).