[go: nahoru, domu]

Add comments and fix a potential leak in LaunchProcess.

I found a need to look at the implementation to confirm that process_handle would never be assigned to if the call failed. So I added a comment for future readers.

While doing so, I found an edge case where I think the process handle could be leaked.

BUG=None
TEST=Ask a random developer whether they must check process_handle if LaunchProcess returns false. If they look at process_util_*.cc, fail.


Review URL: http://codereview.chromium.org/9689081

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126890 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/base/process_util.h b/base/process_util.h
index 1968f8b..d160121 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -259,7 +259,9 @@
   // If true, use an empty string for the desktop name.
   bool empty_desktop_name;
 
-  // If non-NULL, launches the application in that job object.
+  // If non-NULL, launches the application in that job object. The process will
+  // be terminated immediately and LaunchProcess() will fail if assignment to
+  // the job object fails.
   HANDLE job_handle;
 #else
   // If non-NULL, set/unset environment variables.
@@ -322,7 +324,9 @@
 // Launch a process via the command line |cmdline|.
 // See the documentation of LaunchOptions for details on |options|.
 //
-// If |process_handle| is non-NULL, it will be filled in with the
+// Returns true upon success.
+//
+// Upon success, if |process_handle| is non-NULL, it will be filled in with the
 // handle of the launched process.  NOTE: In this case, the caller is
 // responsible for closing the handle so that it doesn't leak!
 // Otherwise, the process handle will be implicitly closed.