public inbox for frysk-cvs@sourceware.org
help / color / mirror / Atom feed
From: cagney@sourceware.org
To: frysk-cvs@sourceware.org
Subject: [SCM]  master: Change frysk.sys.Fork's environ param to a String[]; fixes mem leaks.
Date: Mon, 02 Jun 2008 01:52:00 -0000	[thread overview]
Message-ID: <20080602015204.17620.qmail@sourceware.org> (raw)

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


                 reply	other threads:[~2008-06-02  1:52 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080602015204.17620.qmail@sourceware.org \
    --to=cagney@sourceware.org \
    --cc=frysk-cvs@sourceware.org \
    --cc=frysk@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).