From f6887638b7327f45ff8fa8a29420c48c059182c4 Mon Sep 17 00:00:00 2001
From: Ami Fischman <fischman@chromium.org>
Date: Sat, 3 Mar 2012 19:58:09 -0800
Subject: [PATCH] addressed pcc comments

---
 configure.py           |  4 +---
 src/subprocess.cc      | 19 +++++++++----------
 src/subprocess_test.cc | 12 ++++++++++--
 3 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/configure.py b/configure.py
index 6ea53c1..ca8ba74 100755
--- a/configure.py
+++ b/configure.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 #
-# Copyright 2012 Google Inc. All Rights Reserved.
+# Copyright 2001 Google Inc. All Rights Reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -117,8 +117,6 @@ else:
               '-fno-exceptions',
               '-fvisibility=hidden', '-pipe',
               "'-DNINJA_PYTHON=\"%s\"'" % (options.with_python,)]
-    if platform == 'linux':
-        cflags += ['-DOS_LINUX']
     if options.debug:
         cflags += ['-D_GLIBCXX_DEBUG', '-D_GLIBCXX_DEBUG_PEDANTIC']
     else:
diff --git a/src/subprocess.cc b/src/subprocess.cc
index 3de574b..99de93f 100644
--- a/src/subprocess.cc
+++ b/src/subprocess.cc
@@ -42,12 +42,12 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
   if (pipe(output_pipe) < 0)
     Fatal("pipe: %s", strerror(errno));
   fd_ = output_pipe[0];
-#if !defined(OS_LINUX)
+#if !defined(linux)
   // On linux we use ppoll in DoWork(); elsewhere we use pselect and so must
   // avoid overly-large FDs.
   if (fd_ >= FD_SETSIZE)
     Fatal("pipe: %s", strerror(EMFILE));
-#endif  // !OS_LINUX
+#endif  // !linux
   SetCloseOnExec(fd_);
 
   pid_ = fork();
@@ -182,10 +182,9 @@ Subprocess *SubprocessSet::Add(const string &command) {
   return subprocess;
 }
 
-#ifdef OS_LINUX
+#ifdef linux
 bool SubprocessSet::DoWork() {
-  struct pollfd* fds = new pollfd[running_.size()]();  // XXX: scoped_array?
-  memset(fds, 0, running_.size() * sizeof(struct pollfd));
+  vector<pollfd> fds;
   nfds_t nfds = 0;
 
   for (vector<Subprocess*>::iterator i = running_.begin();
@@ -193,12 +192,12 @@ bool SubprocessSet::DoWork() {
     int fd = (*i)->fd_;
     if (fd < 0)
       continue;
-    fds[nfds].fd = fd;
-    fds[nfds].events = POLLIN | POLLPRI | POLLRDHUP;
+    pollfd pfd = { fd, POLLIN | POLLPRI | POLLRDHUP, 0 };
+    fds.push_back(pfd);
     ++nfds;
   }
 
-  int ret = ppoll(fds, nfds, NULL, &old_mask_);
+  int ret = ppoll(&fds.front(), nfds, NULL, &old_mask_);
   if (ret == -1) {
     if (errno != EINTR) {
       perror("ninja: ppoll");
@@ -230,7 +229,7 @@ bool SubprocessSet::DoWork() {
   return false;
 }
 
-#else  // OS_LINUX
+#else  // linux
 bool SubprocessSet::DoWork() {
   fd_set set;
   int nfds = 0;
@@ -273,7 +272,7 @@ bool SubprocessSet::DoWork() {
 
   return false;
 }
-#endif  // OS_LINUX
+#endif  // linux
 
 Subprocess* SubprocessSet::NextFinished() {
   if (finished_.empty())
diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
index d7ecb33..c155012 100644
--- a/src/subprocess_test.cc
+++ b/src/subprocess_test.cc
@@ -142,11 +142,19 @@ TEST_F(SubprocessTest, SetWithMulti) {
   }
 }
 
-#ifdef OS_LINUX
+#ifdef linux
 TEST_F(SubprocessTest, SetWithLots) {
   // Arbitrary big number; needs to be over 1024 to confirm we're no longer
   // hostage to pselect.
   const size_t kNumProcs = 1025;
+
+  // Make sure [ulimit -n] isn't going to stop us from working.
+  rlimit rlim;
+  ASSERT_EQ(0, getrlimit(RLIMIT_NOFILE, &rlim));
+  ASSERT_GT(rlim.rlim_cur, kNumProcs)
+      << "Raise [ulimit -n] well above " << kNumProcs
+      << " to make this test go";
+
   vector<Subprocess*> procs;
   for (size_t i = 0; i < kNumProcs; ++i) {
     Subprocess* subproc = subprocs_.Add("/bin/echo");
@@ -161,4 +169,4 @@ TEST_F(SubprocessTest, SetWithLots) {
   }
   ASSERT_EQ(kNumProcs, subprocs_.finished_.size());
 }
-#endif
+#endif  // linux
-- 
2.7.4