Merge "adb: keep file flags in fdevent_install."

adb/adb_utils.cpp

@@ -229,3 +229,19 @@ bool parse_host_and_port(const std::string& address,
 std::string perror_str(const char* msg) {
     return android::base::StringPrintf("%s: %s", msg, strerror(errno));
 }
+
+#if !defined(_WIN32)
+bool set_file_block_mode(int fd, bool block) {
+    int flags = fcntl(fd, F_GETFL, 0);
+    if (flags == -1) {
+        PLOG(ERROR) << "failed to fcntl(F_GETFL) for fd " << fd;
+        return false;
+    }
+    flags = block ? (flags & ~O_NONBLOCK) : (flags | O_NONBLOCK);
+    if (fcntl(fd, F_SETFL, flags) != 0) {
+        PLOG(ERROR) << "failed to fcntl(F_SETFL) for fd " << fd << ", flags " << flags;
+        return false;
+    }
+    return true;
+}
+#endif

adb/adb_utils.h

@@ -46,4 +46,6 @@ bool parse_host_and_port(const std::string& address,
 
 std::string perror_str(const char* msg);
 
+bool set_file_block_mode(int fd, bool block);
+
 #endif

adb/adb_utils_test.cpp

@@ -202,3 +202,19 @@ TEST(adb_utils, mkdirs) {
   ASSERT_EQ(0, chdir(td.path)) << strerror(errno);
   test_mkdirs(std::string("relative/subrel/file"));
 }
+
+#if !defined(_WIN32)
+TEST(adb_utils, set_file_block_mode) {
+  int fd = adb_open("/dev/null", O_RDWR | O_APPEND);
+  ASSERT_GE(fd, 0);
+  int flags = fcntl(fd, F_GETFL, 0);
+  ASSERT_EQ(O_RDWR | O_APPEND, (flags & (O_RDWR | O_APPEND)));
+  ASSERT_TRUE(set_file_block_mode(fd, false));
+  int new_flags = fcntl(fd, F_GETFL, 0);
+  ASSERT_EQ(flags | O_NONBLOCK, new_flags);
+  ASSERT_TRUE(set_file_block_mode(fd, true));
+  new_flags = fcntl(fd, F_GETFL, 0);
+  ASSERT_EQ(flags, new_flags);
+  ASSERT_EQ(0, adb_close(fd));
+}
+#endif

adb/fdevent.cpp

@@ -36,6 +36,7 @@
 
 #include "adb_io.h"
 #include "adb_trace.h"
+#include "adb_utils.h"
 
 #if !ADB_HOST
 // This socket is used when a subproc shell service exists.
@@ -124,11 +125,11 @@ void fdevent_install(fdevent* fde, int fd, fd_func func, void* arg) {
     fde->fd = fd;
     fde->func = func;
     fde->arg = arg;
-    if (fcntl(fd, F_SETFL, O_NONBLOCK) != 0) {
+    if (!set_file_block_mode(fd, false)) {
         // Here is not proper to handle the error. If it fails here, some error is
         // likely to be detected by poll(), then we can let the callback function
         // to handle it.
-        LOG(ERROR) << "failed to fcntl(" << fd << ") to be nonblock";
+        LOG(ERROR) << "failed to set non-blocking mode for fd " << fd;
     }
     auto pair = g_poll_node_map.emplace(fde->fd, PollNode(fde));
     CHECK(pair.second) << "install existing fd " << fd;

adb/fdevent_test.cpp

@@ -18,14 +18,14 @@
 
 #include <gtest/gtest.h>
 
+#include <pthread.h>
+#include <signal.h>
+
 #include <limits>
 #include <queue>
 #include <string>
 #include <vector>
 
-#include <pthread.h>
-#include <signal.h>
-
 #include "adb_io.h"
 
 class FdHandler {

adb/jdwp_service.cpp

@@ -27,6 +27,7 @@
 #include <unistd.h>
 
 #include "adb.h"
+#include "adb_utils.h"
 
 /* here's how these things work.
 
@@ -343,7 +344,6 @@ jdwp_process_event( int  socket, unsigned  events, void*  _proc )
             struct iovec     iov;
             char             dummy = '!';
             char             buffer[sizeof(struct cmsghdr) + sizeof(int)];
-            int flags;
 
             iov.iov_base       = &dummy;
             iov.iov_len        = 1;
@@ -361,18 +361,8 @@ jdwp_process_event( int  socket, unsigned  events, void*  _proc )
             cmsg->cmsg_type  = SCM_RIGHTS;
             ((int*)CMSG_DATA(cmsg))[0] = fd;
 
-            flags = fcntl(proc->socket,F_GETFL,0);
-
-            if (flags == -1) {
-                D("failed to get cntl flags for socket %d: %s",
-                  proc->pid, strerror(errno));
-                goto CloseProcess;
-
-            }
-
-            if (fcntl(proc->socket, F_SETFL, flags & ~O_NONBLOCK) == -1) {
-                D("failed to remove O_NONBLOCK flag for socket %d: %s",
-                  proc->pid, strerror(errno));
+            if (!set_file_block_mode(proc->socket, true)) {
+                VLOG(JDWP) << "failed to set blocking mode for fd " << proc->socket;
                 goto CloseProcess;
             }
 
@@ -395,9 +385,8 @@ jdwp_process_event( int  socket, unsigned  events, void*  _proc )
             for (n = 1; n < proc->out_count; n++)
                 proc->out_fds[n-1] = proc->out_fds[n];
 
-            if (fcntl(proc->socket, F_SETFL, flags) == -1) {
-                D("failed to set O_NONBLOCK flag for socket %d: %s",
-                  proc->pid, strerror(errno));
+            if (!set_file_block_mode(proc->socket, false)) {
+                VLOG(JDWP) << "failed to set non-blocking mode for fd " << proc->socket;
                 goto CloseProcess;
             }
 

adb/shell_service.cpp

@@ -77,9 +77,9 @@
 
 #define TRACE_TAG SHELL
 
-#include "shell_service.h"
+#include "sysdeps.h"
 
-#if !ADB_HOST
+#include "shell_service.h"
 
 #include <errno.h>
 #include <pty.h>
@@ -95,7 +95,7 @@
 #include "adb.h"
 #include "adb_io.h"
 #include "adb_trace.h"
-#include "sysdeps.h"
+#include "adb_utils.h"
 
 namespace {
 
@@ -327,9 +327,8 @@ bool Subprocess::ForkAndExec() {
         // the pipe fills up.
         for (int fd : {stdinout_sfd_.fd(), stderr_sfd_.fd()}) {
             if (fd >= 0) {
-                int flags = fcntl(fd, F_GETFL, 0);
-                if (flags < 0 || fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0) {
-                    PLOG(ERROR) << "error making FD " << fd << " non-blocking";
+                if (!set_file_block_mode(fd, false)) {
+                    LOG(ERROR) << "failed to set non-blocking mode for fd " << fd;
                     return false;
                 }
             }
@@ -624,5 +623,3 @@ int StartSubprocess(const char *name, SubprocessType type,
       subprocess->local_socket_fd(), subprocess->pid());
     return subprocess->local_socket_fd();
 }
-
-#endif  // !ADB_HOST