[PATCH 1/2] Use a newtype for remote file descriptor

[Tom Tromey <tromey@adacore.com>]

I was digging through the remote fd code and got a little confused at
some point about what kind of fd was used where.

It seemed better to me to avoid any confusion by making the remote
file descriptor a newtype, i.e., incompatible with 'int'.

I limited the change to the "public" API.  That is, I let the file
descriptor as used by the actual target implementation methods remain
"int".

This found one bug, namely that sparc64-tdep.c assumed that 0 was an
invalid value for a target fd.

The solib-rocm.c changes are best-effort, as I can't compile this
file.
---
 gdb/gdb_bfd.c      |  8 +++----
 gdb/solib-rocm.c   | 28 +++++++++++------------
 gdb/sparc64-tdep.c | 18 +++++++--------
 gdb/target.c       | 65 ++++++++++++++++++++++++++++--------------------------
 gdb/target.h       | 26 ++++++++++++++--------
 5 files changed, 78 insertions(+), 67 deletions(-)

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 1933166b5fb..c53ccce9aa1 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -365,7 +365,7 @@ gdb_bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size,
 
 struct target_fileio_stream : public gdb_bfd_iovec_base
 {
-  target_fileio_stream (bfd *nbfd, int fd)
+  target_fileio_stream (bfd *nbfd, remote_fd fd)
     : m_bfd (nbfd),
       m_fd (fd)
   {
@@ -384,7 +384,7 @@ struct target_fileio_stream : public gdb_bfd_iovec_base
   bfd *m_bfd;
 
   /* The file descriptor.  */
-  int m_fd;
+  remote_fd m_fd;
 };
 
 /* Wrapper for target_fileio_open suitable for use as a helper
@@ -394,7 +394,7 @@ static target_fileio_stream *
 gdb_bfd_iovec_fileio_open (struct bfd *abfd, inferior *inf, bool warn_if_slow)
 {
   const char *filename = bfd_get_filename (abfd);
-  int fd;
+  remote_fd fd;
   fileio_error target_errno;
 
   gdb_assert (is_target_filename (filename));
@@ -403,7 +403,7 @@ gdb_bfd_iovec_fileio_open (struct bfd *abfd, inferior *inf, bool warn_if_slow)
 			   filename + strlen (TARGET_SYSROOT_PREFIX),
 			   FILEIO_O_RDONLY, 0, warn_if_slow,
 			   &target_errno);
-  if (fd == -1)
+  if (fd == remote_fd::INVALID)
     {
       errno = fileio_error_to_host (target_errno);
       bfd_set_error (bfd_error_system_call);
diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c
index 29169417682..6e05913696a 100644
--- a/gdb/solib-rocm.c
+++ b/gdb/solib-rocm.c
@@ -45,26 +45,26 @@ struct rocm_solib_fd_cache
      Open the file FILENAME if it is not already opened, reuse the existing file
      descriptor otherwise.
 
-     On error -1 is returned, and TARGET_ERRNO is set.  */
-  int open (const std::string &filename, fileio_error *target_errno);
+     On error remote_fd::INVALID is returned, and TARGET_ERRNO is set.  */
+  remote_fd open (const std::string &filename, fileio_error *target_errno);
 
   /* Decrement the reference count to FD and close FD if the reference count
      reaches 0.
 
      On success, return 0.  On error, return -1 and set TARGET_ERRNO.  */
-  int close (int fd, fileio_error *target_errno);
+  int close (remote_fd fd, fileio_error *target_errno);
 
 private:
   struct refcnt_fd
   {
-    refcnt_fd (int fd, int refcnt) : fd (fd), refcnt (refcnt) {}
+    refcnt_fd (remote_fd fd, int refcnt) : fd (fd), refcnt (refcnt) {}
 
     DISABLE_COPY_AND_ASSIGN (refcnt_fd);
 
     refcnt_fd (refcnt_fd &&) = default;
     refcnt_fd &operator=(refcnt_fd &&other) = default;
 
-    int fd = -1;
+    remote_fd fd = remote_fd::INVALID;
     int refcnt = 0;
   };
 
@@ -72,7 +72,7 @@ struct rocm_solib_fd_cache
   gdb::unordered_string_map<refcnt_fd> m_cache;
 };
 
-int
+remote_fd
 rocm_solib_fd_cache::open (const std::string &filename,
 			   fileio_error *target_errno)
 {
@@ -80,10 +80,10 @@ rocm_solib_fd_cache::open (const std::string &filename,
   if (it == m_cache.end ())
     {
       /* The file is not yet opened on the target.  */
-      int fd
+      remote_fd fd
 	= target_fileio_open (m_inferior, filename.c_str (), FILEIO_O_RDONLY,
 			      false, 0, target_errno);
-      if (fd != -1)
+      if (fd != remote_fd::INVALID)
 	m_cache.emplace (std::piecewise_construct,
 			 std::forward_as_tuple (filename),
 			 std::forward_as_tuple (fd, 1));
@@ -100,7 +100,7 @@ rocm_solib_fd_cache::open (const std::string &filename,
 }
 
 int
-rocm_solib_fd_cache::close (int fd, fileio_error *target_errno)
+rocm_solib_fd_cache::close (remote_fd fd, fileio_error *target_errno)
 {
   using cache_val = gdb::unordered_string_map<refcnt_fd>::value_type;
   auto it
@@ -349,7 +349,7 @@ struct rocm_code_object_stream_file final : rocm_code_object_stream
 {
   DISABLE_COPY_AND_ASSIGN (rocm_code_object_stream_file);
 
-  rocm_code_object_stream_file (inferior *inf, int fd, ULONGEST offset,
+  rocm_code_object_stream_file (inferior *inf, remote_fd fd, ULONGEST offset,
 				ULONGEST size);
 
   file_ptr read (bfd *abfd, void *buf, file_ptr size,
@@ -365,7 +365,7 @@ struct rocm_code_object_stream_file final : rocm_code_object_stream
   inferior *m_inf;
 
   /* The target file descriptor for this stream.  */
-  int m_fd;
+  remote_fd m_fd;
 
   /* The offset of the ELF file image in the target file.  */
   ULONGEST m_offset;
@@ -376,7 +376,7 @@ struct rocm_code_object_stream_file final : rocm_code_object_stream
 };
 
 rocm_code_object_stream_file::rocm_code_object_stream_file
-  (inferior *inf, int fd, ULONGEST offset, ULONGEST size)
+  (inferior *inf, remote_fd fd, ULONGEST offset, ULONGEST size)
   : m_inf (inf), m_fd (fd), m_offset (offset), m_size (size)
 {
 }
@@ -588,9 +588,9 @@ rocm_bfd_iovec_open (bfd *abfd, inferior *inferior)
 	{
 	  auto info = get_solib_info (inferior);
 	  fileio_error target_errno;
-	  int fd = info->fd_cache.open (decoded_path, &target_errno);
+	  remote_fd fd = info->fd_cache.open (decoded_path, &target_errno);
 
-	  if (fd == -1)
+	  if (fd == remote_fd::INVALID)
 	    {
 	      errno = fileio_error_to_host (target_errno);
 	      bfd_set_error (bfd_error_system_call);
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 15600a640fd..6919a1c37d3 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -101,7 +101,7 @@ struct adi_stat_t
   int max_version;
 
   /* ADI version tag file.  */
-  int tag_fd = 0;
+  remote_fd tag_fd = remote_fd::INVALID;
 
   /* ADI availability check has been done.  */
   bool checked_avail = false;
@@ -176,7 +176,7 @@ sparc64_forget_process (pid_t pid)
     {
       if ((*it).pid == pid)
 	{
-	  if ((*it).stat.tag_fd > 0)
+	  if ((*it).stat.tag_fd != remote_fd::INVALID)
 	    target_fileio_close ((*it).stat.tag_fd, &target_errno);
 	  adi_proc_list.erase_after (pit);
 	  break;
@@ -275,13 +275,13 @@ adi_convert_byte_count (CORE_ADDR naddr, int nbytes, CORE_ADDR locl)
    K * adi_blksz, encoded as 1 version tag per byte.  The allowed
    version tag values are between 0 and adi_stat.max_version.  */
 
-static int
-adi_tag_fd (void)
+static remote_fd
+adi_tag_fd ()
 {
   pid_t pid = inferior_ptid.pid ();
   sparc64_adi_info *proc = get_adi_info_proc (pid);
 
-  if (proc->stat.tag_fd != 0)
+  if (proc->stat.tag_fd != remote_fd::INVALID)
     return proc->stat.tag_fd;
 
   char cl_name[MAX_PROC_NAME_SIZE];
@@ -338,8 +338,8 @@ adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
 static int
 adi_read_versions (CORE_ADDR vaddr, size_t size, gdb_byte *tags)
 {
-  int fd = adi_tag_fd ();
-  if (fd == -1)
+  remote_fd fd = adi_tag_fd ();
+  if (fd == remote_fd::INVALID)
     return -1;
 
   if (!adi_is_addr_mapped (vaddr, size))
@@ -359,8 +359,8 @@ adi_read_versions (CORE_ADDR vaddr, size_t size, gdb_byte *tags)
 static int
 adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
 {
-  int fd = adi_tag_fd ();
-  if (fd == -1)
+  remote_fd fd = adi_tag_fd ();
+  if (fd == remote_fd::INVALID)
     return -1;
 
   if (!adi_is_addr_mapped (vaddr, size))
diff --git a/gdb/target.c b/gdb/target.c
index 4990e20d8d6..cfdf339b013 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3125,7 +3125,7 @@ fileio_handles_invalidate_target (target_ops *targ)
 
 /* Acquire a target fileio file descriptor.  */
 
-static int
+static remote_fd
 acquire_fileio_fd (target_ops *target, int target_fd)
 {
   /* Search for closed handles to reuse.  */
@@ -3148,24 +3148,24 @@ acquire_fileio_fd (target_ops *target, int target_fd)
 
   /* Return its index, and start the next lookup at
      the next index.  */
-  return lowest_closed_fd++;
+  return remote_fd (lowest_closed_fd++);
 }
 
 /* Release a target fileio file descriptor.  */
 
 static void
-release_fileio_fd (int fd, fileio_fh_t *fh)
+release_fileio_fd (remote_fd fd, fileio_fh_t *fh)
 {
   fh->target_fd = -1;
-  lowest_closed_fd = std::min (lowest_closed_fd, fd);
+  lowest_closed_fd = std::min (lowest_closed_fd, int (fd));
 }
 
 /* Return a pointer to the fileio_fhandle_t corresponding to FD.  */
 
 static fileio_fh_t *
-fileio_fd_to_fh (int fd)
+fileio_fd_to_fh (remote_fd fd)
 {
-  return &fileio_fhandles[fd];
+  return &fileio_fhandles[int (fd)];
 }
 
 
@@ -3239,7 +3239,7 @@ target_ops::fileio_readlink (struct inferior *inf, const char *filename,
 
 /* See target.h.  */
 
-int
+remote_fd
 target_fileio_open (struct inferior *inf, const char *filename,
 		    int flags, int mode, bool warn_if_slow, fileio_error *target_errno)
 {
@@ -3251,25 +3251,27 @@ target_fileio_open (struct inferior *inf, const char *filename,
       if (fd == -1 && *target_errno == FILEIO_ENOSYS)
 	continue;
 
+      remote_fd result;
       if (fd < 0)
-	fd = -1;
+	result = remote_fd::INVALID;
       else
-	fd = acquire_fileio_fd (t, fd);
+	result = acquire_fileio_fd (t, fd);
 
       target_debug_printf_nofunc ("target_fileio_open (%d,%s,0x%x,0%o,%d) = %d (%d)",
-			   inf == NULL ? 0 : inf->num, filename, flags, mode,
-			   warn_if_slow, fd, fd != -1 ? 0 : *target_errno);
-      return fd;
+				  inf == NULL ? 0 : inf->num, filename,
+				  flags, mode, warn_if_slow, int (fd),
+				  int (fd) != -1 ? 0 : *target_errno);
+      return result;
     }
 
   *target_errno = FILEIO_ENOSYS;
-  return -1;
+  return remote_fd::INVALID;
 }
 
 /* See target.h.  */
 
 int
-target_fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
+target_fileio_pwrite (remote_fd fd, const gdb_byte *write_buf, int len,
 		      ULONGEST offset, fileio_error *target_errno)
 {
   fileio_fh_t *fh = fileio_fd_to_fh (fd);
@@ -3283,16 +3285,16 @@ target_fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
     ret = fh->target->fileio_pwrite (fh->target_fd, write_buf,
 				     len, offset, target_errno);
 
-  target_debug_printf_nofunc ("target_fileio_pwrite (%d,...,%d,%s) = %d (%d)", fd,
-		       len, pulongest (offset), ret,
-		       ret != -1 ? 0 : *target_errno);
+  target_debug_printf_nofunc ("target_fileio_pwrite (%d,...,%d,%s) = %d (%d)",
+			      int (fd), len, pulongest (offset), ret,
+			      ret != -1 ? 0 : *target_errno);
   return ret;
 }
 
 /* See target.h.  */
 
 int
-target_fileio_pread (int fd, gdb_byte *read_buf, int len,
+target_fileio_pread (remote_fd fd, gdb_byte *read_buf, int len,
 		     ULONGEST offset, fileio_error *target_errno)
 {
   fileio_fh_t *fh = fileio_fd_to_fh (fd);
@@ -3306,15 +3308,16 @@ target_fileio_pread (int fd, gdb_byte *read_buf, int len,
     ret = fh->target->fileio_pread (fh->target_fd, read_buf,
 				    len, offset, target_errno);
 
-  target_debug_printf_nofunc ("target_fileio_pread (%d,...,%d,%s) = %d (%d)", fd, len,
-		       pulongest (offset), ret, ret != -1 ? 0 : *target_errno);
+  target_debug_printf_nofunc ("target_fileio_pread (%d,...,%d,%s) = %d (%d)",
+			      int (fd), len, pulongest (offset), ret,
+			      ret != -1 ? 0 : *target_errno);
   return ret;
 }
 
 /* See target.h.  */
 
 int
-target_fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno)
+target_fileio_fstat (remote_fd fd, struct stat *sb, fileio_error *target_errno)
 {
   fileio_fh_t *fh = fileio_fd_to_fh (fd);
   int ret = -1;
@@ -3326,8 +3329,8 @@ target_fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno)
   else
     ret = fh->target->fileio_fstat (fh->target_fd, sb, target_errno);
 
-  target_debug_printf_nofunc ("target_fileio_fstat (%d) = %d (%d)", fd, ret,
-		       ret != -1 ? 0 : *target_errno);
+  target_debug_printf_nofunc ("target_fileio_fstat (%d) = %d (%d)",
+			      int (fd), ret, ret != -1 ? 0 : *target_errno);
   return ret;
 }
 
@@ -3357,7 +3360,7 @@ target_fileio_lstat (struct inferior *inf, const char *filename,
 /* See target.h.  */
 
 int
-target_fileio_close (int fd, fileio_error *target_errno)
+target_fileio_close (remote_fd fd, fileio_error *target_errno)
 {
   fileio_fh_t *fh = fileio_fd_to_fh (fd);
   int ret = -1;
@@ -3374,8 +3377,8 @@ target_fileio_close (int fd, fileio_error *target_errno)
       release_fileio_fd (fd, fh);
     }
 
-  target_debug_printf_nofunc ("target_fileio_close (%d) = %d (%d)", fd, ret,
-		       ret != -1 ? 0 : *target_errno);
+  target_debug_printf_nofunc ("target_fileio_close (%d) = %d (%d)",
+			      int (fd), ret, ret != -1 ? 0 : *target_errno);
   return ret;
 }
 
@@ -3432,14 +3435,14 @@ target_fileio_readlink (struct inferior *inf, const char *filename,
 class scoped_target_fd
 {
 public:
-  explicit scoped_target_fd (int fd) noexcept
+  explicit scoped_target_fd (remote_fd fd) noexcept
     : m_fd (fd)
   {
   }
 
   ~scoped_target_fd ()
   {
-    if (m_fd >= 0)
+    if (m_fd != remote_fd::INVALID)
       {
 	fileio_error target_errno;
 
@@ -3449,13 +3452,13 @@ class scoped_target_fd
 
   DISABLE_COPY_AND_ASSIGN (scoped_target_fd);
 
-  int get () const noexcept
+  remote_fd get () const noexcept
   {
     return m_fd;
   }
 
 private:
-  int m_fd;
+  remote_fd m_fd;
 };
 
 /* Read target file FILENAME, in the filesystem as seen by INF.  If
@@ -3477,7 +3480,7 @@ target_fileio_read_alloc_1 (struct inferior *inf, const char *filename,
 
   scoped_target_fd fd (target_fileio_open (inf, filename, FILEIO_O_RDONLY,
 					   0700, false, &target_errno));
-  if (fd.get () == -1)
+  if (fd.get () == remote_fd::INVALID)
     return -1;
 
   /* Start by reading up to 4K at a time.  The target will throttle
diff --git a/gdb/target.h b/gdb/target.h
index 4db9c3c7d98..87cf8acac28 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2250,33 +2250,41 @@ extern int target_search_memory (CORE_ADDR start_addr,
 
 extern bool target_filesystem_is_local ();
 
+/* A remote file descriptor is just an integer, but we use a separate
+   type to avoid confusion with local file descriptors.  */
+enum class remote_fd : int
+{
+  INVALID = -1,
+};
+
 /* Open FILENAME on the target, in the filesystem as seen by INF,
    using FLAGS and MODE.  If INF is NULL, use the filesystem seen by
    the debugger (GDB or, for remote targets, the remote stub).  Return
    a target file descriptor, or -1 if an error occurs (and set
    *TARGET_ERRNO).  If WARN_IF_SLOW is true, print a warning message
    if the file is being accessed over a link that may be slow.  */
-extern int target_fileio_open (struct inferior *inf,
-			       const char *filename, int flags,
-			       int mode, bool warn_if_slow,
-			       fileio_error *target_errno);
+extern remote_fd target_fileio_open (struct inferior *inf,
+				     const char *filename, int flags,
+				     int mode, bool warn_if_slow,
+				     fileio_error *target_errno);
 
 /* Write up to LEN bytes from WRITE_BUF to FD on the target.
    Return the number of bytes written, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
-extern int target_fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
-				 ULONGEST offset, fileio_error *target_errno);
+extern int target_fileio_pwrite (remote_fd fd, const gdb_byte *write_buf,
+				 int len, ULONGEST offset,
+				 fileio_error *target_errno);
 
 /* Read up to LEN bytes FD on the target into READ_BUF.
    Return the number of bytes read, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
-extern int target_fileio_pread (int fd, gdb_byte *read_buf, int len,
+extern int target_fileio_pread (remote_fd fd, gdb_byte *read_buf, int len,
 				ULONGEST offset, fileio_error *target_errno);
 
 /* Get information about the file opened as FD on the target
    and put it in SB.  Return 0 on success, or -1 if an error
    occurs (and set *TARGET_ERRNO).  */
-extern int target_fileio_fstat (int fd, struct stat *sb,
+extern int target_fileio_fstat (remote_fd fd, struct stat *sb,
 				fileio_error *target_errno);
 
 /* Get information about the file at FILENAME on the target and put it in
@@ -2289,7 +2297,7 @@ extern int target_fileio_lstat (struct inferior *inf, const char *filename,
 
 /* Close FD on the target.  Return 0, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
-extern int target_fileio_close (int fd, fileio_error *target_errno);
+extern int target_fileio_close (remote_fd fd, fileio_error *target_errno);
 
 /* Unlink FILENAME on the target, in the filesystem as seen by INF.
    If INF is NULL, use the filesystem seen by the debugger (GDB or,

-- 
2.53.0