am 641fd2e4: am ebce1470: Merge "adb: fix adb_close() vs. unix_close() usage"

* commit '641fd2e443607aadd2cb98177308241cb633dcbd':
  adb: fix adb_close() vs. unix_close() usage
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 9c1ead5..ff6a499 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -115,7 +115,7 @@
     dup2(fd, STDOUT_FILENO);
     dup2(fd, STDERR_FILENO);
     fprintf(stderr, "--- adb starting (pid %d) ---\n", getpid());
-    adb_close(fd);
+    unix_close(fd);
 }
 #endif
 
diff --git a/adb/adb_auth_host.cpp b/adb/adb_auth_host.cpp
index 61a3777..e878f8b 100644
--- a/adb/adb_auth_host.cpp
+++ b/adb/adb_auth_host.cpp
@@ -420,6 +420,9 @@
     strcat(path, ".pub");
 
     // TODO(danalbert): ReadFileToString
+    // Note that on Windows, load_file() does not do CR/LF translation, but
+    // ReadFileToString() uses the C Runtime which uses CR/LF translation by
+    // default (by is overridable with _setmode()).
     unsigned size;
     char* file_data = reinterpret_cast<char*>(load_file(path, &size));
     if (file_data == nullptr) {
diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp
index c0612cd..78ab3f6 100644
--- a/adb/daemon/main.cpp
+++ b/adb/daemon/main.cpp
@@ -226,7 +226,7 @@
         return;
     }
     dup2(fd, STDIN_FILENO);
-    adb_close(fd);
+    unix_close(fd);
 }
 
 int main(int argc, char** argv) {
diff --git a/adb/remount_service.cpp b/adb/remount_service.cpp
index 7a3b89a..3a4d2da 100644
--- a/adb/remount_service.cpp
+++ b/adb/remount_service.cpp
@@ -58,7 +58,7 @@
 
     int OFF = 0;
     bool result = (ioctl(fd, BLKROSET, &OFF) != -1);
-    adb_close(fd);
+    unix_close(fd);
     return result;
 }
 
diff --git a/adb/services.cpp b/adb/services.cpp
index a9edbe5..2fd75ab 100644
--- a/adb/services.cpp
+++ b/adb/services.cpp
@@ -252,7 +252,7 @@
     *pid = forkpty(&ptm, pts_name, nullptr, nullptr);
     if (*pid == -1) {
         printf("- fork failed: %s -\n", strerror(errno));
-        adb_close(ptm);
+        unix_close(ptm);
         return -1;
     }
 
@@ -263,7 +263,7 @@
         if (pts == -1) {
             fprintf(stderr, "child failed to open pseudo-term slave %s: %s\n",
                     pts_name, strerror(errno));
-            adb_close(ptm);
+            unix_close(ptm);
             exit(-1);
         }
 
@@ -272,16 +272,16 @@
             termios tattr;
             if (tcgetattr(pts, &tattr) == -1) {
                 fprintf(stderr, "tcgetattr failed: %s\n", strerror(errno));
-                adb_close(pts);
-                adb_close(ptm);
+                unix_close(pts);
+                unix_close(ptm);
                 exit(-1);
             }
 
             cfmakeraw(&tattr);
             if (tcsetattr(pts, TCSADRAIN, &tattr) == -1) {
                 fprintf(stderr, "tcsetattr failed: %s\n", strerror(errno));
-                adb_close(pts);
-                adb_close(ptm);
+                unix_close(pts);
+                unix_close(ptm);
                 exit(-1);
             }
         }
@@ -290,8 +290,8 @@
         dup2(pts, STDOUT_FILENO);
         dup2(pts, STDERR_FILENO);
 
-        adb_close(pts);
-        adb_close(ptm);
+        unix_close(pts);
+        unix_close(ptm);
 
         execl(cmd, cmd, arg0, arg1, nullptr);
         fprintf(stderr, "- exec '%s' failed: %s (%d) -\n",
diff --git a/adb/sysdeps.h b/adb/sysdeps.h
index 0aa1570..b7c16d9 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -125,6 +125,7 @@
 #undef   mkdir
 #define  mkdir  ___xxx_mkdir
 
+// See the comments for the !defined(_WIN32) versions of adb_*().
 extern int  adb_open(const char*  path, int  options);
 extern int  adb_creat(const char*  path, int  mode);
 extern int  adb_read(int  fd, void* buf, int len);
@@ -133,6 +134,7 @@
 extern int  adb_shutdown(int  fd);
 extern int  adb_close(int  fd);
 
+// See the comments for the !defined(_WIN32) version of unix_close().
 static __inline__ int  unix_close(int fd)
 {
     return close(fd);
@@ -140,11 +142,13 @@
 #undef   close
 #define  close   ____xxx_close
 
+// See the comments for the !defined(_WIN32) version of unix_read().
 extern int  unix_read(int  fd, void*  buf, size_t  len);
 
 #undef   read
 #define  read  ___xxx_read
 
+// See the comments for the !defined(_WIN32) version of unix_write().
 static __inline__  int  unix_write(int  fd, const void*  buf, size_t  len)
 {
     return write(fd, buf, len);
@@ -152,11 +156,13 @@
 #undef   write
 #define  write  ___xxx_write
 
+// See the comments for the !defined(_WIN32) version of adb_open_mode().
 static __inline__ int  adb_open_mode(const char* path, int options, int mode)
 {
     return adb_open(path, options);
 }
 
+// See the comments for the !defined(_WIN32) version of unix_open().
 static __inline__ int  unix_open(const char*  path, int options,...)
 {
     if ((options & O_CREAT) == 0)
@@ -314,6 +320,15 @@
     fcntl( fd, F_SETFD, FD_CLOEXEC );
 }
 
+// Open a file and return a file descriptor that may be used with unix_read(),
+// unix_write(), unix_close(), but not adb_read(), adb_write(), adb_close().
+//
+// On Unix, this is based on open(), so the file descriptor is a real OS file
+// descriptor, but the Windows implementation (in sysdeps_win32.cpp) returns a
+// file descriptor that can only be used with C Runtime APIs (which are wrapped
+// by unix_read(), unix_write(), unix_close()). Also, the C Runtime has
+// configurable CR/LF translation which defaults to text mode, but is settable
+// with _setmode().
 static __inline__ int  unix_open(const char*  path, int options,...)
 {
     if ((options & O_CREAT) == 0)
@@ -331,12 +346,21 @@
     }
 }
 
+// Similar to the two-argument adb_open(), but takes a mode parameter for file
+// creation. See adb_open() for more info.
 static __inline__ int  adb_open_mode( const char*  pathname, int  options, int  mode )
 {
     return TEMP_FAILURE_RETRY( open( pathname, options, mode ) );
 }
 
 
+// Open a file and return a file descriptor that may be used with adb_read(),
+// adb_write(), adb_close(), but not unix_read(), unix_write(), unix_close().
+//
+// On Unix, this is based on open(), but the Windows implementation (in
+// sysdeps_win32.cpp) uses Windows native file I/O and bypasses the C Runtime
+// and its CR/LF translation. The returned file descriptor should be used with
+// adb_read(), adb_write(), adb_close(), etc.
 static __inline__ int  adb_open( const char*  pathname, int  options )
 {
     int  fd = TEMP_FAILURE_RETRY( open( pathname, options ) );
@@ -355,6 +379,9 @@
 #undef   shutdown
 #define  shutdown   ____xxx_shutdown
 
+// Closes a file descriptor that came from adb_open() or adb_open_mode(), but
+// not designed to take a file descriptor from unix_open(). See the comments
+// for adb_open() for more info.
 static __inline__ int  adb_close(int fd)
 {
     return close(fd);
@@ -419,6 +446,14 @@
 #undef   accept
 #define  accept  ___xxx_accept
 
+// Operate on a file descriptor returned from unix_open() or a well-known file
+// descriptor such as STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO.
+//
+// On Unix, unix_read(), unix_write(), unix_close() map to adb_read(),
+// adb_write(), adb_close() (which all map to Unix system calls), but the
+// Windows implementations (in the ifdef above and in sysdeps_win32.cpp) call
+// into the C Runtime and its configurable CR/LF translation (which is settable
+// via _setmode()).
 #define  unix_read   adb_read
 #define  unix_write  adb_write
 #define  unix_close  adb_close
diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp
index a21272f..50c99f1 100644
--- a/adb/sysdeps_win32.cpp
+++ b/adb/sysdeps_win32.cpp
@@ -172,14 +172,15 @@
 static  int          _win32_fh_count;
 
 static FH
-_fh_from_int( int   fd )
+_fh_from_int( int   fd, const char*   func )
 {
     FH  f;
 
     fd -= WIN32_FH_BASE;
 
     if (fd < 0 || fd >= _win32_fh_count) {
-        D( "_fh_from_int: invalid fd %d\n", fd + WIN32_FH_BASE );
+        D( "_fh_from_int: invalid fd %d passed to %s\n", fd + WIN32_FH_BASE,
+           func );
         errno = EBADF;
         return NULL;
     }
@@ -187,7 +188,8 @@
     f = &_win32_fhs[fd];
 
     if (f->used == 0) {
-        D( "_fh_from_int: invalid fd %d\n", fd + WIN32_FH_BASE );
+        D( "_fh_from_int: invalid fd %d passed to %s\n", fd + WIN32_FH_BASE,
+           func );
         errno = EBADF;
         return NULL;
     }
@@ -427,7 +429,7 @@
 
 int  adb_read(int  fd, void* buf, int len)
 {
-    FH     f = _fh_from_int(fd);
+    FH     f = _fh_from_int(fd, __func__);
 
     if (f == NULL) {
         return -1;
@@ -439,7 +441,7 @@
 
 int  adb_write(int  fd, const void*  buf, int  len)
 {
-    FH     f = _fh_from_int(fd);
+    FH     f = _fh_from_int(fd, __func__);
 
     if (f == NULL) {
         return -1;
@@ -451,7 +453,7 @@
 
 int  adb_lseek(int  fd, int  pos, int  where)
 {
-    FH     f = _fh_from_int(fd);
+    FH     f = _fh_from_int(fd, __func__);
 
     if (!f) {
         return -1;
@@ -463,7 +465,7 @@
 
 int  adb_shutdown(int  fd)
 {
-    FH   f = _fh_from_int(fd);
+    FH   f = _fh_from_int(fd, __func__);
 
     if (!f || f->clazz != &_fh_socket_class) {
         D("adb_shutdown: invalid fd %d\n", fd);
@@ -478,7 +480,7 @@
 
 int  adb_close(int  fd)
 {
-    FH   f = _fh_from_int(fd);
+    FH   f = _fh_from_int(fd, __func__);
 
     if (!f) {
         return -1;
@@ -763,7 +765,7 @@
 #undef accept
 int  adb_socket_accept(int  serverfd, struct sockaddr*  addr, socklen_t  *addrlen)
 {
-    FH   serverfh = _fh_from_int(serverfd);
+    FH   serverfh = _fh_from_int(serverfd, __func__);
     FH   fh;
 
     if ( !serverfh || serverfh->clazz != &_fh_socket_class ) {
@@ -792,7 +794,7 @@
 
 int  adb_setsockopt( int  fd, int  level, int  optname, const void*  optval, socklen_t  optlen )
 {
-    FH   fh = _fh_from_int(fd);
+    FH   fh = _fh_from_int(fd, __func__);
 
     if ( !fh || fh->clazz != &_fh_socket_class ) {
         D("adb_setsockopt: invalid fd %d\n", fd);
@@ -1386,7 +1388,7 @@
 static void
 event_looper_hook( EventLooper  looper, int  fd, int  events )
 {
-    FH          f = _fh_from_int(fd);
+    FH          f = _fh_from_int(fd, __func__);
     EventHook  *pnode;
     EventHook   node;
 
@@ -1418,7 +1420,7 @@
 static void
 event_looper_unhook( EventLooper  looper, int  fd, int  events )
 {
-    FH          fh    = _fh_from_int(fd);
+    FH          fh    = _fh_from_int(fd, __func__);
     EventHook  *pnode = event_looper_find_p( looper, fh );
     EventHook   node  = *pnode;
 
@@ -3036,7 +3038,7 @@
     }
 }
 
-// Called by 'adb shell' command to read from stdin.
+// Called by 'adb shell' and 'adb exec-in' to read from stdin.
 int unix_read(int fd, void* buf, size_t len) {
     if ((fd == STDIN_FILENO) && (_console_handle != NULL)) {
         // If it is a request to read from stdin, and stdin_raw_init() has been
@@ -3046,8 +3048,12 @@
         return _console_read(_console_handle, buf, len);
     } else {
         // Just call into C Runtime which can read from pipes/files and which
-        // can do LF/CR translation.
+        // can do LF/CR translation (which is overridable with _setmode()).
+        // Undefine the macro that is set in sysdeps.h which bans calls to
+        // plain read() in favor of unix_read() or adb_read().
+#pragma push_macro("read")
 #undef read
         return read(fd, buf, len);
+#pragma pop_macro("read")
     }
 }
diff --git a/adb/usb_linux.cpp b/adb/usb_linux.cpp
index 1e5d5c8..c6f712b 100644
--- a/adb/usb_linux.cpp
+++ b/adb/usb_linux.cpp
@@ -168,13 +168,13 @@
                 continue;
             }
 
-            desclength = adb_read(fd, devdesc, sizeof(devdesc));
+            desclength = unix_read(fd, devdesc, sizeof(devdesc));
             bufend = bufptr + desclength;
 
                 // should have device and configuration descriptors, and atleast two endpoints
             if (desclength < USB_DT_DEVICE_SIZE + USB_DT_CONFIG_SIZE) {
                 D("desclength %zu is too small\n", desclength);
-                adb_close(fd);
+                unix_close(fd);
                 continue;
             }
 
@@ -182,7 +182,7 @@
             bufptr += USB_DT_DEVICE_SIZE;
 
             if((device->bLength != USB_DT_DEVICE_SIZE) || (device->bDescriptorType != USB_DT_DEVICE)) {
-                adb_close(fd);
+                unix_close(fd);
                 continue;
             }
 
@@ -195,7 +195,7 @@
             bufptr += USB_DT_CONFIG_SIZE;
             if (config->bLength != USB_DT_CONFIG_SIZE || config->bDescriptorType != USB_DT_CONFIG) {
                 D("usb_config_descriptor not found\n");
-                adb_close(fd);
+                unix_close(fd);
                 continue;
             }
 
@@ -303,7 +303,7 @@
                 }
             } // end of while
 
-            adb_close(fd);
+            unix_close(fd);
         } // end of devdir while
         closedir(devdir);
     } //end of busdir while
@@ -555,7 +555,7 @@
     h->prev = 0;
     h->next = 0;
 
-    adb_close(h->desc);
+    unix_close(h->desc);
     D("-- usb closed %p (fd = %d) --\n", h, h->desc);
     adb_mutex_unlock(&usb_lock);
 
@@ -618,7 +618,7 @@
         if (ioctl(usb->desc, USBDEVFS_CLAIMINTERFACE, &interface) != 0) {
             D("[ usb ioctl(%d, USBDEVFS_CLAIMINTERFACE) failed: %s]\n",
               usb->desc, strerror(errno));
-            adb_close(usb->desc);
+            unix_close(usb->desc);
             free(usb);
             return;
         }
diff --git a/adb/usb_linux_client.cpp b/adb/usb_linux_client.cpp
index 63bb1c7..d34c454 100644
--- a/adb/usb_linux_client.cpp
+++ b/adb/usb_linux_client.cpp
@@ -201,7 +201,7 @@
     int n;
 
     D("about to write (fd=%d, len=%d)\n", h->fd, len);
-    n = adb_write(h->fd, data, len);
+    n = unix_write(h->fd, data, len);
     if(n != len) {
         D("ERROR: fd = %d, n = %d, errno = %d (%s)\n",
             h->fd, n, errno, strerror(errno));
@@ -216,7 +216,7 @@
     int n;
 
     D("about to read (fd=%d, len=%d)\n", h->fd, len);
-    n = adb_read(h->fd, data, len);
+    n = unix_read(h->fd, data, len);
     if(n != len) {
         D("ERROR: fd = %d, n = %d, errno = %d (%s)\n",
             h->fd, n, errno, strerror(errno));
@@ -230,7 +230,7 @@
 {
     D("usb_kick\n");
     adb_mutex_lock(&h->lock);
-    adb_close(h->fd);
+    unix_close(h->fd);
     h->fd = -1;
 
     // notify usb_adb_open_thread that we are disconnected