You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2020/05/01 16:54:05 UTC

[mesos] 02/03: Moved documentation into os/exec.hpp interface.

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 9befebac5f054657686c9a6490603afe9b7f8ce1
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Thu Apr 30 16:08:59 2020 -0400

    Moved documentation into os/exec.hpp interface.
    
    The os/exec.hpp wrapper provides a single place to read the interface
    and documentation for both POSIX and Windows, this makes it easier
    for us to document the interface and OS differences that a caller
    needs to know about in one place.
    
    Review: https://reviews.apache.org/r/72303/
---
 3rdparty/stout/include/stout/os/exec.hpp         | 91 ++++++++++++++++++++++++
 3rdparty/stout/include/stout/os/posix/exec.hpp   | 13 ----
 3rdparty/stout/include/stout/os/windows/exec.hpp |  9 ---
 3 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/3rdparty/stout/include/stout/os/exec.hpp b/3rdparty/stout/include/stout/os/exec.hpp
index 4ad130b..686e7a1 100644
--- a/3rdparty/stout/include/stout/os/exec.hpp
+++ b/3rdparty/stout/include/stout/os/exec.hpp
@@ -21,4 +21,95 @@
 #include <stout/os/posix/exec.hpp>
 #endif // __WINDOWS__
 
+namespace os {
+
+// Forks a subprocess and executes the provided file with the provided
+// arguments.
+//
+// Blocks until the subprocess terminates and returns the exit code of
+// the subprocess, or `None` if an error occurred (e.g., fork / exec /
+// waitpid or the Windows equivalents failed).
+//
+// POSIX: this function is async signal safe. We return an
+// `Option<int>` instead of a `Try<int>`, because although `Try`
+// does not dynamically allocate, `Error` uses `std::string`,
+// which is not async signal safe.
+//
+// Windows: Note that on Windows, processes are started using
+// a string command line, and each process does its own parsing
+// of that command line into arguments. This function will quote
+// and escape the arguments compatible with any programs that
+// use `CommandLineToArgvW` (this is the most common way, and any
+// programs that use libc-style main with an arguments array will
+// use this under the covers). However, some programs, notably
+// cmd.exe have their own approach for parsing quotes / arguments
+// that are not compatible with CommandLineToArgvW, and therefore
+// should not be used with this function!
+//
+// TODO(bmahler): Add a windows only overload that takes a single
+// string command line (to support cmd.exe and others with non-
+// CommandLineToArgvW parsing). See MESOS-10093.
+inline Option<int> spawn(
+    const std::string& file,
+    const std::vector<std::string>& arguments);
+
+
+// This wrapper allows a caller to call `execvp` on both POSIX
+// and Windows systems.
+//
+// Windows: In order to emulate `execvp`, this function forks
+// another subprocess to execute with the provided arguments,
+// and once the subprocess terminates, the parent process will
+// in turn exit with the same exit code. Note that on Windows,
+// processes are started using a string command line, and each
+// process does its own parsing of that command line into
+// arguments. This function will quote and escape the arguments
+// compatible with any programs that use `CommandLineToArgvW`
+// (this is the most common way, and any programs that use
+// libc-style main with an arguments array will use this under
+// the covers). However, some programs, notably cmd.exe have
+// their own approach for parsing quotes / arguments that are
+// not compatible with CommandLineToArgvW, and therefore should
+// not be used with this function!
+//
+// TODO(bmahler): Probably we shouldn't provide this windows
+// emulation and should instead have the caller use windows
+// subprocess functions directly?
+inline int execvp(const char* file, char* const argv[]);
+
+
+// This function is a portable version of execvpe ('p' means searching
+// executable from PATH and 'e' means setting environments). We add
+// this function because it is not available on all POSIX systems.
+//
+// POSIX: This function is not thread safe. It is supposed to be used
+// only after fork (when there is only one thread). This function is
+// async signal safe.
+//
+// Windows: In order to emulate `execvpe`, this function forks
+// another subprocess to execute with the provided arguments,
+// and once the subprocess terminates, the parent process will
+// in turn exit with the same exit code. Note that on Windows,
+// processes are started using a string command line, and each
+// process does its own parsing of that command line into
+// arguments. This function will quote and escape the arguments
+// compatible with any programs that use `CommandLineToArgvW`
+// (this is the most common way, and any programs that use
+// libc-style main with an arguments array will use this under
+// the covers). However, some programs, notably cmd.exe have
+// their own approach for parsing quotes / arguments that are
+// not compatible with CommandLineToArgvW, and therefore should
+// not be used with this function!
+//
+// NOTE: This function can accept `Argv` and `Envp` constructs
+// via their implicit type conversions, but on Windows, it cannot
+// accept the os::raw forms.
+//
+// TODO(bmahler): Probably we shouldn't provide this windows
+// emulation and should instead have the caller use windows
+// subprocess functions directly?
+inline int execvpe(const char* file, char** argv, char** envp);
+
+} // namespace os {
+
 #endif // __STOUT_OS_EXEC_HPP__
diff --git a/3rdparty/stout/include/stout/os/posix/exec.hpp b/3rdparty/stout/include/stout/os/posix/exec.hpp
index dd0f42d..3057b45 100644
--- a/3rdparty/stout/include/stout/os/posix/exec.hpp
+++ b/3rdparty/stout/include/stout/os/posix/exec.hpp
@@ -29,12 +29,6 @@
 
 namespace os {
 
-// Executes a command by calling "<command> <arguments...>", and returns after
-// the command has been completed. Returns the exit code on success and `None`
-// on error (e.g., fork/exec/waitpid failed). This function is async signal
-// safe. We return an `Option<int>` instead of a `Try<int>`, because although
-// `Try` does not dynamically allocate, `Error` uses `std::string`, which is
-// not async signal safe.
 inline Option<int> spawn(
     const std::string& file,
     const std::vector<std::string>& arguments)
@@ -67,13 +61,6 @@ inline int execvp(const char* file, char* const argv[])
 }
 
 
-// This function is a portable version of execvpe ('p' means searching
-// executable from PATH and 'e' means setting environments). We add
-// this function because it is not available on all systems.
-//
-// NOTE: This function is not thread safe. It is supposed to be used
-// only after fork (when there is only one thread). This function is
-// async signal safe.
 inline int execvpe(const char* file, char** argv, char** envp)
 {
   char** saved = os::raw::environment();
diff --git a/3rdparty/stout/include/stout/os/windows/exec.hpp b/3rdparty/stout/include/stout/os/windows/exec.hpp
index 8c6f10b..21e1d15 100644
--- a/3rdparty/stout/include/stout/os/windows/exec.hpp
+++ b/3rdparty/stout/include/stout/os/windows/exec.hpp
@@ -409,9 +409,6 @@ inline Try<ProcessData> create_process(
 } // namespace windows {
 
 
-// Executes a command by calling "<command> <arguments...>", and
-// returns after the command has been completed. Returns the process exit
-// code on success and `None` on error.
 inline Option<int> spawn(
     const std::string& command,
     const std::vector<std::string>& arguments,
@@ -442,9 +439,6 @@ inline Option<int> spawn(
 }
 
 
-// In order to emulate the semantics of `execvp`, `os::spawn` waits for the new
-// process to exit, and returns its error code, which is propagated back to the
-// parent via `exit` here.
 inline int execvp(
     const std::string& file,
     const std::vector<std::string>& argv)
@@ -454,9 +448,6 @@ inline int execvp(
 }
 
 
-// NOTE: This function can accept `Argv` and `Envp` constructs through their
-// explicit type conversions, but unlike the POSIX implementations, it cannot
-// accept the raw forms.
 inline int execvpe(
     const std::string& file,
     const std::vector<std::string>& argv,