[PATCH] D16546: [sanitizers] extracted process management functions

[Alexey Samsonov via llvm-commits]

samsonov accepted this revision.
samsonov added a comment.
This revision is now accepted and ready to land.

LGTM after addressing existing comments. Thanks!


================
Comment at: lib/sanitizer_common/sanitizer_common.h:284
@@ +283,3 @@
+// If *_fd parameters are not kInvalidFd their corresponding input/output
+// streams would be redirect to the file. The files would always be closed
+// in parent process even in case of an error.
----------------
filcab wrote:
> Nit: s/would/will/g
"will be redirected"

================
Comment at: lib/sanitizer_common/sanitizer_common.h:290
@@ -282,1 +289,3 @@
+// Checks if specified process is still running
+bool IsProcessRunning(int pid);
 
----------------
filcab wrote:
> Should this argument (and StartSubprocess' return value) be something else, since, as Alexey said, "on Windows fd_t is void*"?
> Probably either a typedef or just a uptr?
process id is not fd_t. But I agree that uptr might be more appropriate. We use it for ReportFile::fd_pid, for example.

================
Comment at: lib/sanitizer_common/sanitizer_posix_libcdep.cc:324
@@ -322,1 +323,3 @@
 
+int StartSubprocess(const char *program, const char *const argv[], fd_t stdin,
+                    fd_t stdout, fd_t stderr) {
----------------
Please match arg names to function declaration. Also (sorry for not being clear) I meant stdin_fd, stdout_fd, stderr_fd to avoid confusing with predefined values of the same name in headers.


http://reviews.llvm.org/D16546