[PATCH v2 0/3] Stronger typing for remote file i/o

[PATCH v2 0/3] Stronger typing for remote file i/o

[Tom Tromey]

This short series adds stronger typing to the remote file i/o APIs, by
replacing some integers with enums.  This prevents mixing host and
remote-protocol values, and also differentiates between host and
remote file descriptors.

Regression tested on x86-64 Fedora 43.

Signed-off-by: Tom Tromey <tromey@adacore.com>
---
Changes in v2:
- Renamed remote_fd to target_fd
- Various other minor changes per review
- Added warn_if_slow patch
- Link to v1: https://inbox.sourceware.org/gdb-patches/20260225-target-fd-newtype-v1-0-e04af6692ccb@adacore.com

---
Tom Tromey (3):
      Use a newtype for remote file descriptor
      Use enum types for remote fileio flags
      Use bool for "warn_if_slow"

 gdb/gdb_bfd.c        |  8 ++---
 gdb/inf-child.c      |  4 +--
 gdb/inf-child.h      |  4 +--
 gdb/linux-nat.c      |  4 +--
 gdb/linux-nat.h      |  4 +--
 gdb/remote-fileio.c  | 91 +++++++++-------------------------------------------
 gdb/remote.c         | 31 ++++++++++--------
 gdb/solib-rocm.c     | 32 +++++++++---------
 gdb/sparc64-tdep.c   | 24 +++++++-------
 gdb/target.c         | 86 ++++++++++++++++++++++++++-----------------------
 gdb/target.h         | 32 +++++++++++-------
 gdbserver/hostio.cc  |  4 +--
 gdbsupport/fileio.cc | 20 ++++++------
 gdbsupport/fileio.h  | 85 +++++++++++++++++++++++++++---------------------
 14 files changed, 201 insertions(+), 228 deletions(-)
---
base-commit: adbc0c55421dd5e31bb1903512dfb8f5211ee5b0
change-id: 20260225-target-fd-newtype-b5c3e3ad31b7

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

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

[Tom Tromey]

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.

New in v2:
- solib-rocm.c compiles now
- Renamed to target_fd (including scoped_target_fd)

Approved-By: Simon Marchi <simon.marchi@efficios.com>
---
 gdb/gdb_bfd.c      |  8 +++---
 gdb/remote.c       | 12 ++++-----
 gdb/solib-rocm.c   | 32 +++++++++++------------
 gdb/sparc64-tdep.c | 18 ++++++-------
 gdb/target.c       | 76 ++++++++++++++++++++++++++++--------------------------
 gdb/target.h       | 26 ++++++++++++-------
 6 files changed, 92 insertions(+), 80 deletions(-)

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 1933166b5fb..70494745c82 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, target_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;
+  target_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;
+  target_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 == target_fd::INVALID)
     {
       errno = fileio_error_to_host (target_errno);
       bfd_set_error (bfd_error_system_call);
diff --git a/gdb/remote.c b/gdb/remote.c
index 22f584f0c57..a5fb16381b1 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -13869,15 +13869,15 @@ remote_hostio_error (fileio_error errnum)
 
 /* A RAII wrapper around a remote file descriptor.  */
 
-class scoped_remote_fd
+class scoped_target_fd
 {
 public:
-  scoped_remote_fd (remote_target *remote, int fd)
+  scoped_target_fd (remote_target *remote, int fd)
     : m_remote (remote), m_fd (fd)
   {
   }
 
-  ~scoped_remote_fd ()
+  ~scoped_target_fd ()
   {
     if (m_fd != -1)
       {
@@ -13907,7 +13907,7 @@ class scoped_remote_fd
       }
   }
 
-  DISABLE_COPY_AND_ASSIGN (scoped_remote_fd);
+  DISABLE_COPY_AND_ASSIGN (scoped_target_fd);
 
   /* Release ownership of the file descriptor, and return it.  */
   ATTRIBUTE_UNUSED_RESULT int release () noexcept
@@ -13956,7 +13956,7 @@ remote_target::remote_file_put (const char *local_file, const char *remote_file,
   if (file == NULL)
     perror_with_name (local_file);
 
-  scoped_remote_fd fd
+  scoped_target_fd fd
     (this, remote_hostio_open (NULL,
 			       remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT
 					     | FILEIO_O_TRUNC),
@@ -14044,7 +14044,7 @@ remote_target::remote_file_get (const char *remote_file, const char *local_file,
   int bytes, io_size;
   ULONGEST offset;
 
-  scoped_remote_fd fd
+  scoped_target_fd fd
     (this, remote_hostio_open (NULL,
 			       remote_file, FILEIO_O_RDONLY, 0, 0,
 			       &remote_errno));
diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c
index 29169417682..d9ae4294c98 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 target_fd::INVALID is returned, and TARGET_ERRNO is set.  */
+  target_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 (target_fd fd, fileio_error *target_errno);
 
 private:
   struct refcnt_fd
   {
-    refcnt_fd (int fd, int refcnt) : fd (fd), refcnt (refcnt) {}
+    refcnt_fd (target_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;
+    target_fd fd = target_fd::INVALID;
     int refcnt = 0;
   };
 
@@ -72,7 +72,7 @@ struct rocm_solib_fd_cache
   gdb::unordered_string_map<refcnt_fd> m_cache;
 };
 
-int
+target_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
+      target_fd fd
 	= target_fileio_open (m_inferior, filename.c_str (), FILEIO_O_RDONLY,
-			      false, 0, target_errno);
-      if (fd != -1)
+			      0, false, target_errno);
+      if (fd != target_fd::INVALID)
 	m_cache.emplace (std::piecewise_construct,
 			 std::forward_as_tuple (filename),
 			 std::forward_as_tuple (fd, 1));
@@ -94,13 +94,13 @@ rocm_solib_fd_cache::open (const std::string &filename,
       /* The file is already opened.  Increment the refcnt and return the
 	 already opened FD.  */
       it->second.refcnt++;
-      gdb_assert (it->second.fd != -1);
+      gdb_assert (it->second.fd != target_fd::INVALID);
       return it->second.fd;
     }
 }
 
 int
-rocm_solib_fd_cache::close (int fd, fileio_error *target_errno)
+rocm_solib_fd_cache::close (target_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, target_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;
+  target_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, target_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);
+	  target_fd fd = info->fd_cache.open (decoded_path, &target_errno);
 
-	  if (fd == -1)
+	  if (fd == target_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..e4d06eeef61 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;
+  target_fd tag_fd = target_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 != target_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 target_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 != target_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)
+  target_fd fd = adi_tag_fd ();
+  if (fd == target_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)
+  target_fd fd = adi_tag_fd ();
+  if (fd == target_fd::INVALID)
     return -1;
 
   if (!adi_is_addr_mapped (vaddr, size))
diff --git a/gdb/target.c b/gdb/target.c
index 4990e20d8d6..5264b0fb04b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3125,8 +3125,8 @@ fileio_handles_invalidate_target (target_ops *targ)
 
 /* Acquire a target fileio file descriptor.  */
 
-static int
-acquire_fileio_fd (target_ops *target, int target_fd)
+static target_fd
+acquire_fileio_fd (target_ops *target, int underlying_fd)
 {
   /* Search for closed handles to reuse.  */
   for (; lowest_closed_fd < fileio_fhandles.size (); lowest_closed_fd++)
@@ -3139,33 +3139,33 @@ acquire_fileio_fd (target_ops *target, int target_fd)
 
   /* Push a new handle if no closed handles were found.  */
   if (lowest_closed_fd == fileio_fhandles.size ())
-    fileio_fhandles.push_back (fileio_fh_t {target, target_fd});
+    fileio_fhandles.push_back (fileio_fh_t {target, underlying_fd});
   else
-    fileio_fhandles[lowest_closed_fd] = {target, target_fd};
+    fileio_fhandles[lowest_closed_fd] = {target, underlying_fd};
 
   /* Should no longer be marked closed.  */
   gdb_assert (!fileio_fhandles[lowest_closed_fd].is_closed ());
 
   /* Return its index, and start the next lookup at
      the next index.  */
-  return lowest_closed_fd++;
+  return target_fd (lowest_closed_fd++);
 }
 
 /* Release a target fileio file descriptor.  */
 
 static void
-release_fileio_fd (int fd, fileio_fh_t *fh)
+release_fileio_fd (target_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 (target_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
+target_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,28 @@ target_fileio_open (struct inferior *inf, const char *filename,
       if (fd == -1 && *target_errno == FILEIO_ENOSYS)
 	continue;
 
+      target_fd result;
       if (fd < 0)
-	fd = -1;
+	result = target_fd::INVALID;
       else
-	fd = 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;
+	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, int (result),
+	 result != target_fd::INVALID ? 0 : *target_errno);
+      return result;
     }
 
   *target_errno = FILEIO_ENOSYS;
-  return -1;
+  return target_fd::INVALID;
 }
 
 /* See target.h.  */
 
 int
-target_fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
+target_fileio_pwrite (target_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 +3286,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 (target_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 +3309,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 (target_fd fd, struct stat *sb, fileio_error *target_errno)
 {
   fileio_fh_t *fh = fileio_fd_to_fh (fd);
   int ret = -1;
@@ -3326,8 +3330,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 +3361,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 (target_fd fd, fileio_error *target_errno)
 {
   fileio_fh_t *fh = fileio_fd_to_fh (fd);
   int ret = -1;
@@ -3374,8 +3378,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 +3436,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 (target_fd fd) noexcept
     : m_fd (fd)
   {
   }
 
   ~scoped_target_fd ()
   {
-    if (m_fd >= 0)
+    if (m_fd != target_fd::INVALID)
       {
 	fileio_error target_errno;
 
@@ -3449,13 +3453,13 @@ class scoped_target_fd
 
   DISABLE_COPY_AND_ASSIGN (scoped_target_fd);
 
-  int get () const noexcept
+  target_fd get () const noexcept
   {
     return m_fd;
   }
 
 private:
-  int m_fd;
+  target_fd m_fd;
 };
 
 /* Read target file FILENAME, in the filesystem as seen by INF.  If
@@ -3477,7 +3481,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 () == target_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..f76ac09ee82 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 target_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 target_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 (target_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 (target_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 (target_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 (target_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

[PATCH v2 2/3] Use enum types for remote fileio flags

[Tom Tromey]

This changes gdbsupport/fileio.h to use enums with underlying types
for the various constants -- open flags, modes, and lseek flags.
These types replace #defines that were previously used.

Then, this fixes all the users of these flags.  This found a few bugs.
Some of these were pedantic (using the constant 0700 where perhaps
FILEIO_S_IRWXU would be more precise), but sparc64-tdep.c confused
host and remote flags.

Also, I believe solib-rocm.c had a couple of arguments in the wrong
order.  Once again, the solib-rocm.c changes are best-effort, as I
can't compile this file.

I also found that gdb/remote-fileio.c had essentially duplicated some
code from gdbsupport.  This patch removes the duplicates.

New in v2:
- The lseek enum is anonymous, the type itself isn't used, just
  the constants
---
 gdb/inf-child.c      |  4 +--
 gdb/inf-child.h      |  4 +--
 gdb/linux-nat.c      |  4 +--
 gdb/linux-nat.h      |  4 +--
 gdb/remote-fileio.c  | 91 +++++++++-------------------------------------------
 gdb/remote.c         | 19 ++++++-----
 gdb/sparc64-tdep.c   |  6 ++--
 gdb/target.c         | 12 ++++---
 gdb/target.h         | 10 +++---
 gdbserver/hostio.cc  |  4 +--
 gdbsupport/fileio.cc | 20 ++++++------
 gdbsupport/fileio.h  | 85 +++++++++++++++++++++++++++---------------------
 12 files changed, 112 insertions(+), 151 deletions(-)

diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 934d8c2ba09..22e52686d5e 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -233,8 +233,8 @@ inf_child_target::pid_to_exec_file (int pid)
 
 int
 inf_child_target::fileio_open (struct inferior *inf, const char *filename,
-			       int flags, int mode, int warn_if_slow,
-			       fileio_error *target_errno)
+			       fileio_open_flags flags, fileio_mode_flags mode,
+			       int warn_if_slow, fileio_error *target_errno)
 {
   int nat_flags;
   mode_t nat_mode;
diff --git a/gdb/inf-child.h b/gdb/inf-child.h
index bb18f08dbcf..7d2d4eab68e 100644
--- a/gdb/inf-child.h
+++ b/gdb/inf-child.h
@@ -74,8 +74,8 @@ class inf_child_target
   const char *pid_to_exec_file (int pid) override;
 
   int fileio_open (struct inferior *inf, const char *filename,
-		   int flags, int mode, int warn_if_slow,
-		   fileio_error *target_errno) override;
+		   fileio_open_flags flags, fileio_mode_flags mode,
+		   int warn_if_slow, fileio_error *target_errno) override;
   int fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
 		     ULONGEST offset, fileio_error *target_errno) override;
   int fileio_pread (int fd, gdb_byte *read_buf, int len,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 958b367f4ea..22dbfbc31f9 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4572,8 +4572,8 @@ linux_nat_fileio_pid_of (struct inferior *inf)
 
 int
 linux_nat_target::fileio_open (struct inferior *inf, const char *filename,
-			       int flags, int mode, int warn_if_slow,
-			       fileio_error *target_errno)
+			       fileio_open_flags flags, fileio_mode_flags mode,
+			       int warn_if_slow, fileio_error *target_errno)
 {
   int nat_flags;
   mode_t nat_mode;
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 4641615d531..2d2ccbc2371 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -100,8 +100,8 @@ class linux_nat_target : public inf_ptrace_target
   bool filesystem_is_local () override;
 
   int fileio_open (struct inferior *inf, const char *filename,
-		   int flags, int mode, int warn_if_slow,
-		   fileio_error *target_errno) override;
+		   fileio_open_flags flags, fileio_mode_flags mode,
+		   int warn_if_slow, fileio_error *target_errno) override;
 
   std::optional<std::string>
     fileio_readlink (struct inferior *inf,
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index 459f249d37b..30afc3737ab 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -120,78 +120,6 @@ remote_fileio_close_target_fd (int target_fd)
     remote_fio_data.fd_map[target_fd] = FIO_FD_INVALID;
 }
 
-static int
-remote_fileio_oflags_to_host (long flags)
-{
-  int hflags = 0;
-
-  if (flags & FILEIO_O_CREAT)
-    hflags |= O_CREAT;
-  if (flags & FILEIO_O_EXCL)
-    hflags |= O_EXCL;
-  if (flags & FILEIO_O_TRUNC)
-    hflags |= O_TRUNC;
-  if (flags & FILEIO_O_APPEND)
-    hflags |= O_APPEND;
-  if (flags & FILEIO_O_RDONLY)
-    hflags |= O_RDONLY;
-  if (flags & FILEIO_O_WRONLY)
-    hflags |= O_WRONLY;
-  if (flags & FILEIO_O_RDWR)
-    hflags |= O_RDWR;
-/* On systems supporting binary and text mode, always open files in
-   binary mode.  */
-#ifdef O_BINARY
-  hflags |= O_BINARY;
-#endif
-  return hflags;
-}
-
-static mode_t
-remote_fileio_mode_to_host (long mode, int open_call)
-{
-  mode_t hmode = 0;
-
-  if (!open_call)
-    {
-      if (mode & FILEIO_S_IFREG)
-	hmode |= S_IFREG;
-      if (mode & FILEIO_S_IFDIR)
-	hmode |= S_IFDIR;
-      if (mode & FILEIO_S_IFCHR)
-	hmode |= S_IFCHR;
-    }
-  if (mode & FILEIO_S_IRUSR)
-    hmode |= S_IRUSR;
-  if (mode & FILEIO_S_IWUSR)
-    hmode |= S_IWUSR;
-  if (mode & FILEIO_S_IXUSR)
-    hmode |= S_IXUSR;
-#ifdef S_IRGRP
-  if (mode & FILEIO_S_IRGRP)
-    hmode |= S_IRGRP;
-#endif
-#ifdef S_IWGRP
-  if (mode & FILEIO_S_IWGRP)
-    hmode |= S_IWGRP;
-#endif
-#ifdef S_IXGRP
-  if (mode & FILEIO_S_IXGRP)
-    hmode |= S_IXGRP;
-#endif
-  if (mode & FILEIO_S_IROTH)
-    hmode |= S_IROTH;
-#ifdef S_IWOTH
-  if (mode & FILEIO_S_IWOTH)
-    hmode |= S_IWOTH;
-#endif
-#ifdef S_IXOTH
-  if (mode & FILEIO_S_IXOTH)
-    hmode |= S_IXOTH;
-#endif
-  return hmode;
-}
-
 static int
 remote_fileio_seek_flag_to_host (long num, int *flag)
 {
@@ -391,14 +319,23 @@ remote_fileio_func_open (remote_target *remote, char *buf)
       remote_fileio_ioerror (remote);
       return;
     }
-  flags = remote_fileio_oflags_to_host (num);
+  if (fileio_to_host_openflags ((enum fileio_open_flag) num, &flags))
+    {
+      remote_fileio_ioerror (remote);
+      return;
+    }
+
   /* 3. Parameter: open mode */
   if (remote_fileio_extract_int (&buf, &num))
     {
       remote_fileio_ioerror (remote);
       return;
     }
-  mode = remote_fileio_mode_to_host (num, 1);
+  if (fileio_to_host_mode (fileio_mode_flag (num), &mode))
+    {
+      remote_fileio_ioerror (remote);
+      return;
+    }
 
   /* Request pathname.  */
   pathname = (char *) alloca (length);
@@ -1233,8 +1170,10 @@ remote_fileio_to_host_ulong (fio_ulong_t fnum)
 static mode_t
 remote_fileio_to_host_mode (fio_mode_t fnum)
 {
-  return remote_fileio_mode_to_host (remote_fileio_to_host_uint (fnum),
-				     0);
+  mode_t result;
+  ULONGEST conv = remote_fileio_to_host_uint (fnum);
+  fileio_to_host_mode ((fileio_mode_flag) conv, &result);
+  return result;
 }
 
 /* Unpack an fio_time_t.  */
diff --git a/gdb/remote.c b/gdb/remote.c
index a5fb16381b1..04b24f0a703 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1114,7 +1114,8 @@ class remote_target : public process_stratum_target
 
 
   int fileio_open (struct inferior *inf, const char *filename,
-		   int flags, int mode, int warn_if_slow,
+		   fileio_open_flags flags, fileio_mode_flags mode,
+		   int warn_if_slow,
 		   fileio_error *target_errno) override;
 
   int fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
@@ -1289,8 +1290,8 @@ class remote_target : public process_stratum_target
 				    fileio_error *remote_errno);
   /* We should get rid of this and use fileio_open directly.  */
   int remote_hostio_open (struct inferior *inf, const char *filename,
-			  int flags, int mode, int warn_if_slow,
-			  fileio_error *remote_errno);
+			  fileio_open_flags flags, fileio_mode_flags mode,
+			  int warn_if_slow, fileio_error *remote_errno);
   int remote_hostio_close (int fd, fileio_error *remote_errno);
 
   int remote_hostio_unlink (inferior *inf, const char *filename,
@@ -13400,7 +13401,9 @@ remote_target::remote_hostio_set_filesystem (struct inferior *inf,
 
 int
 remote_target::remote_hostio_open (inferior *inf, const char *filename,
-				   int flags, int mode, int warn_if_slow,
+				   fileio_open_flags flags,
+				   fileio_mode_flags mode,
+				   int warn_if_slow,
 				   fileio_error *remote_errno)
 {
   struct remote_state *rs = get_remote_state ();
@@ -13446,8 +13449,8 @@ remote_target::remote_hostio_open (inferior *inf, const char *filename,
 
 int
 remote_target::fileio_open (struct inferior *inf, const char *filename,
-			    int flags, int mode, int warn_if_slow,
-			    fileio_error *remote_errno)
+			    fileio_open_flags flags, fileio_mode_flags mode,
+			    int warn_if_slow, fileio_error *remote_errno)
 {
   return remote_hostio_open (inf, filename, flags, mode, warn_if_slow,
 			     remote_errno);
@@ -13828,7 +13831,7 @@ remote_target::filesystem_is_local ()
 	     filename is irrelevant, we only care about whether
 	     the stub recognizes the packet or not.  */
 	  fd = remote_hostio_open (NULL, "just probing",
-				   FILEIO_O_RDONLY, 0700, 0,
+				   FILEIO_O_RDONLY, FILEIO_S_IRWXU, 0,
 				   &remote_errno);
 
 	  if (fd >= 0)
@@ -13960,7 +13963,7 @@ remote_target::remote_file_put (const char *local_file, const char *remote_file,
     (this, remote_hostio_open (NULL,
 			       remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT
 					     | FILEIO_O_TRUNC),
-			       0700, 0, &remote_errno));
+			       FILEIO_S_IRWXU, 0, &remote_errno));
   if (fd.get () == -1)
     remote_hostio_error (remote_errno);
 
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index e4d06eeef61..88bc0ba3ad4 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -287,8 +287,10 @@ adi_tag_fd ()
   char cl_name[MAX_PROC_NAME_SIZE];
   snprintf (cl_name, sizeof(cl_name), "/proc/%ld/adi/tags", (long) pid);
   fileio_error target_errno;
-  proc->stat.tag_fd = target_fileio_open (NULL, cl_name, O_RDWR|O_EXCL,
-					  false, 0, &target_errno);
+  proc->stat.tag_fd = target_fileio_open (NULL, cl_name,
+					  FILEIO_O_RDWR | FILEIO_O_EXCL,
+					  0, false,
+					  &target_errno);
   return proc->stat.tag_fd;
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index 5264b0fb04b..611f37c9730 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3176,8 +3176,8 @@ fileio_fd_to_fh (target_fd fd)
 
 int
 target_ops::fileio_open (struct inferior *inf, const char *filename,
-			 int flags, int mode, int warn_if_slow,
-			 fileio_error *target_errno)
+			 fileio_open_flags flags, fileio_mode_flags mode,
+			 int warn_if_slow, fileio_error *target_errno)
 {
   *target_errno = FILEIO_ENOSYS;
   return -1;
@@ -3241,7 +3241,8 @@ target_ops::fileio_readlink (struct inferior *inf, const char *filename,
 
 target_fd
 target_fileio_open (struct inferior *inf, const char *filename,
-		    int flags, int mode, bool warn_if_slow, fileio_error *target_errno)
+		    fileio_open_flags flags, fileio_mode_flags mode,
+		    bool warn_if_slow, fileio_error *target_errno)
 {
   for (target_ops *t = default_fileio_target (); t != NULL; t = t->beneath ())
     {
@@ -3260,7 +3261,7 @@ target_fileio_open (struct inferior *inf, const char *filename,
       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, int (result),
+	 unsigned (flags), unsigned (mode), warn_if_slow, int (result),
 	 result != target_fd::INVALID ? 0 : *target_errno);
       return result;
     }
@@ -3480,7 +3481,8 @@ target_fileio_read_alloc_1 (struct inferior *inf, const char *filename,
   fileio_error target_errno;
 
   scoped_target_fd fd (target_fileio_open (inf, filename, FILEIO_O_RDONLY,
-					   0700, false, &target_errno));
+					   FILEIO_S_IRWXU, false,
+					   &target_errno));
   if (fd.get () == target_fd::INVALID)
     return -1;
 
diff --git a/gdb/target.h b/gdb/target.h
index f76ac09ee82..89f6180b9e5 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -991,8 +991,8 @@ struct target_ops
        target file descriptor, or -1 if an error occurs (and set
        *TARGET_ERRNO).  */
     virtual int fileio_open (struct inferior *inf, const char *filename,
-			     int flags, int mode, int warn_if_slow,
-			     fileio_error *target_errno);
+			     fileio_open_flags flags, fileio_mode_flags mode,
+			     int 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
@@ -2264,8 +2264,10 @@ enum class target_fd : int
    *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 target_fd target_fileio_open (struct inferior *inf,
-				     const char *filename, int flags,
-				     int mode, bool warn_if_slow,
+				     const char *filename,
+				     fileio_open_flags flags,
+				     fileio_mode_flags mode,
+				     bool warn_if_slow,
 				     fileio_error *target_errno);
 
 /* Write up to LEN bytes from WRITE_BUF to FD on the target.
diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc
index a0a0735deb4..b9ca27c2ff4 100644
--- a/gdbserver/hostio.cc
+++ b/gdbserver/hostio.cc
@@ -318,8 +318,8 @@ handle_open (char *own_buf)
       || require_comma (&p)
       || require_int (&p, &fileio_mode)
       || require_end (p)
-      || fileio_to_host_openflags (fileio_flags, &flags)
-      || fileio_to_host_mode (fileio_mode, &mode))
+      || fileio_to_host_openflags (fileio_open_flag (fileio_flags), &flags)
+      || fileio_to_host_mode (fileio_mode_flag (fileio_mode), &mode))
     {
       hostio_packet_error (own_buf);
       return;
diff --git a/gdbsupport/fileio.cc b/gdbsupport/fileio.cc
index aedb67335ec..6d2ffb14544 100644
--- a/gdbsupport/fileio.cc
+++ b/gdbsupport/fileio.cc
@@ -130,26 +130,26 @@ fileio_error_to_host (fileio_error errnum)
 /* See fileio.h.  */
 
 int
-fileio_to_host_openflags (int fileio_open_flags, int *open_flags_p)
+fileio_to_host_openflags (fileio_open_flags fflags, int *open_flags_p)
 {
   int open_flags = 0;
 
-  if (fileio_open_flags & ~FILEIO_O_SUPPORTED)
+  if (fflags & ~FILEIO_O_SUPPORTED)
     return -1;
 
-  if (fileio_open_flags & FILEIO_O_CREAT)
+  if (fflags & FILEIO_O_CREAT)
     open_flags |= O_CREAT;
-  if (fileio_open_flags & FILEIO_O_EXCL)
+  if (fflags & FILEIO_O_EXCL)
     open_flags |= O_EXCL;
-  if (fileio_open_flags & FILEIO_O_TRUNC)
+  if (fflags & FILEIO_O_TRUNC)
     open_flags |= O_TRUNC;
-  if (fileio_open_flags & FILEIO_O_APPEND)
+  if (fflags & FILEIO_O_APPEND)
     open_flags |= O_APPEND;
-  if (fileio_open_flags & FILEIO_O_RDONLY)
+  if (fflags & FILEIO_O_RDONLY)
     open_flags |= O_RDONLY;
-  if (fileio_open_flags & FILEIO_O_WRONLY)
+  if (fflags & FILEIO_O_WRONLY)
     open_flags |= O_WRONLY;
-  if (fileio_open_flags & FILEIO_O_RDWR)
+  if (fflags & FILEIO_O_RDWR)
     open_flags |= O_RDWR;
   /* On systems supporting binary and text mode, always open files
      in binary mode. */
@@ -164,7 +164,7 @@ fileio_to_host_openflags (int fileio_open_flags, int *open_flags_p)
 /* See fileio.h.  */
 
 int
-fileio_to_host_mode (int fileio_mode, mode_t *mode_p)
+fileio_to_host_mode (fileio_mode_flags fileio_mode, mode_t *mode_p)
 {
   mode_t mode = 0;
 
diff --git a/gdbsupport/fileio.h b/gdbsupport/fileio.h
index f7b1bdf6375..bb70e404bba 100644
--- a/gdbsupport/fileio.h
+++ b/gdbsupport/fileio.h
@@ -21,6 +21,7 @@
 #define GDBSUPPORT_FILEIO_H
 
 #include <sys/stat.h>
+#include "enum-flags.h"
 
 /* The following flags are defined to be independent of the host
    as well as the target side implementation of these constants.
@@ -29,42 +30,53 @@
    corresponding implementation dependent constants in one module. */
 
 /* open(2) flags */
-#define FILEIO_O_RDONLY           0x0
-#define FILEIO_O_WRONLY           0x1
-#define FILEIO_O_RDWR             0x2
-#define FILEIO_O_APPEND           0x8
-#define FILEIO_O_CREAT          0x200
-#define FILEIO_O_TRUNC          0x400
-#define FILEIO_O_EXCL           0x800
-#define FILEIO_O_SUPPORTED      (FILEIO_O_RDONLY | FILEIO_O_WRONLY| \
-				 FILEIO_O_RDWR   | FILEIO_O_APPEND| \
-				 FILEIO_O_CREAT  | FILEIO_O_TRUNC| \
-				 FILEIO_O_EXCL)
+enum fileio_open_flag : unsigned
+{
+  FILEIO_O_RDONLY = 0x0,
+  FILEIO_O_WRONLY = 0x1,
+  FILEIO_O_RDWR = 0x2,
+  FILEIO_O_APPEND = 0x8,
+  FILEIO_O_CREAT = 0x200,
+  FILEIO_O_TRUNC = 0x400,
+  FILEIO_O_EXCL = 0x800,
+  FILEIO_O_SUPPORTED = (FILEIO_O_RDONLY | FILEIO_O_WRONLY
+			| FILEIO_O_RDWR | FILEIO_O_APPEND
+			| FILEIO_O_CREAT | FILEIO_O_TRUNC
+			| FILEIO_O_EXCL),
+};
+DEF_ENUM_FLAGS_TYPE (enum fileio_open_flag, fileio_open_flags);
 
 /* mode_t bits */
-#define FILEIO_S_IFREG        0100000
-#define FILEIO_S_IFDIR         040000
-#define FILEIO_S_IFCHR         020000
-#define FILEIO_S_IRUSR           0400
-#define FILEIO_S_IWUSR           0200
-#define FILEIO_S_IXUSR           0100
-#define FILEIO_S_IRWXU           0700
-#define FILEIO_S_IRGRP            040
-#define FILEIO_S_IWGRP            020
-#define FILEIO_S_IXGRP            010
-#define FILEIO_S_IRWXG            070
-#define FILEIO_S_IROTH             04
-#define FILEIO_S_IWOTH             02
-#define FILEIO_S_IXOTH             01
-#define FILEIO_S_IRWXO             07
-#define FILEIO_S_SUPPORTED         (FILEIO_S_IFREG|FILEIO_S_IFDIR|  \
-				    FILEIO_S_IRWXU|FILEIO_S_IRWXG|  \
-				    FILEIO_S_IRWXO)
+enum fileio_mode_flag : unsigned
+{
+  FILEIO_S_IFREG = 0100000,
+  FILEIO_S_IFDIR = 040000,
+  FILEIO_S_IFCHR = 020000,
+  FILEIO_S_IRUSR = 0400,
+  FILEIO_S_IWUSR = 0200,
+  FILEIO_S_IXUSR = 0100,
+  FILEIO_S_IRWXU = 0700,
+  FILEIO_S_IRGRP = 040,
+  FILEIO_S_IWGRP = 020,
+  FILEIO_S_IXGRP = 010,
+  FILEIO_S_IRWXG = 070,
+  FILEIO_S_IROTH = 04,
+  FILEIO_S_IWOTH = 02,
+  FILEIO_S_IXOTH = 01,
+  FILEIO_S_IRWXO = 07,
+  FILEIO_S_SUPPORTED = (FILEIO_S_IFREG | FILEIO_S_IFDIR
+			| FILEIO_S_IRWXU | FILEIO_S_IRWXG
+			| FILEIO_S_IRWXO),
+};
+DEF_ENUM_FLAGS_TYPE (enum fileio_mode_flag, fileio_mode_flags);
 
 /* lseek(2) flags */
-#define FILEIO_SEEK_SET             0
-#define FILEIO_SEEK_CUR             1
-#define FILEIO_SEEK_END             2
+enum
+{
+  FILEIO_SEEK_SET = 0,
+  FILEIO_SEEK_CUR = 1,
+  FILEIO_SEEK_END = 2,
+};
 
 /* errno values */
 enum fileio_error
@@ -144,14 +156,15 @@ extern fileio_error host_to_fileio_error (int error);
 extern int fileio_error_to_host (fileio_error errnum);
 
 /* Convert File-I/O open flags FFLAGS to host format, storing
-   the result in *FLAGS.  Return 0 on success, -1 on error.  */
+   the result in *OPEN_FLAGS_P.  Return 0 on success, -1 on error.  */
 
-extern int fileio_to_host_openflags (int fflags, int *flags);
+extern int fileio_to_host_openflags (fileio_open_flags fflags,
+				     int *open_flags_p);
 
 /* Convert File-I/O mode FMODE to host format, storing
-   the result in *MODE.  Return 0 on success, -1 on error.  */
+   the result in *MODE_P.  Return 0 on success, -1 on error.  */
 
-extern int fileio_to_host_mode (int fmode, mode_t *mode);
+extern int fileio_to_host_mode (fileio_mode_flags fmode, mode_t *mode_p);
 
 /* Pack a host-format integer into a byte buffer in big-endian
    format.  BYTES specifies the size of the integer to pack in

-- 
2.53.0

[PATCH v2 3/3] Use bool for "warn_if_slow"

[Tom Tromey]

A few places still used int for the type of the 'warn_if_slow' flag.
This fixes all these spots.
---
 gdb/inf-child.c | 2 +-
 gdb/inf-child.h | 2 +-
 gdb/linux-nat.c | 2 +-
 gdb/linux-nat.h | 2 +-
 gdb/remote.c    | 8 ++++----
 gdb/target.c    | 2 +-
 gdb/target.h    | 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 22e52686d5e..7983ecb0d92 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -234,7 +234,7 @@ inf_child_target::pid_to_exec_file (int pid)
 int
 inf_child_target::fileio_open (struct inferior *inf, const char *filename,
 			       fileio_open_flags flags, fileio_mode_flags mode,
-			       int warn_if_slow, fileio_error *target_errno)
+			       bool warn_if_slow, fileio_error *target_errno)
 {
   int nat_flags;
   mode_t nat_mode;
diff --git a/gdb/inf-child.h b/gdb/inf-child.h
index 7d2d4eab68e..9fa62664b36 100644
--- a/gdb/inf-child.h
+++ b/gdb/inf-child.h
@@ -75,7 +75,7 @@ class inf_child_target
 
   int fileio_open (struct inferior *inf, const char *filename,
 		   fileio_open_flags flags, fileio_mode_flags mode,
-		   int warn_if_slow, fileio_error *target_errno) override;
+		   bool warn_if_slow, fileio_error *target_errno) override;
   int fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
 		     ULONGEST offset, fileio_error *target_errno) override;
   int fileio_pread (int fd, gdb_byte *read_buf, int len,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 22dbfbc31f9..1d0a4609a3a 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4573,7 +4573,7 @@ linux_nat_fileio_pid_of (struct inferior *inf)
 int
 linux_nat_target::fileio_open (struct inferior *inf, const char *filename,
 			       fileio_open_flags flags, fileio_mode_flags mode,
-			       int warn_if_slow, fileio_error *target_errno)
+			       bool warn_if_slow, fileio_error *target_errno)
 {
   int nat_flags;
   mode_t nat_mode;
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 2d2ccbc2371..0f128892735 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -101,7 +101,7 @@ class linux_nat_target : public inf_ptrace_target
 
   int fileio_open (struct inferior *inf, const char *filename,
 		   fileio_open_flags flags, fileio_mode_flags mode,
-		   int warn_if_slow, fileio_error *target_errno) override;
+		   bool warn_if_slow, fileio_error *target_errno) override;
 
   std::optional<std::string>
     fileio_readlink (struct inferior *inf,
diff --git a/gdb/remote.c b/gdb/remote.c
index 04b24f0a703..7ec4cbf3209 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1115,7 +1115,7 @@ class remote_target : public process_stratum_target
 
   int fileio_open (struct inferior *inf, const char *filename,
 		   fileio_open_flags flags, fileio_mode_flags mode,
-		   int warn_if_slow,
+		   bool warn_if_slow,
 		   fileio_error *target_errno) override;
 
   int fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
@@ -1291,7 +1291,7 @@ class remote_target : public process_stratum_target
   /* We should get rid of this and use fileio_open directly.  */
   int remote_hostio_open (struct inferior *inf, const char *filename,
 			  fileio_open_flags flags, fileio_mode_flags mode,
-			  int warn_if_slow, fileio_error *remote_errno);
+			  bool warn_if_slow, fileio_error *remote_errno);
   int remote_hostio_close (int fd, fileio_error *remote_errno);
 
   int remote_hostio_unlink (inferior *inf, const char *filename,
@@ -13403,7 +13403,7 @@ int
 remote_target::remote_hostio_open (inferior *inf, const char *filename,
 				   fileio_open_flags flags,
 				   fileio_mode_flags mode,
-				   int warn_if_slow,
+				   bool warn_if_slow,
 				   fileio_error *remote_errno)
 {
   struct remote_state *rs = get_remote_state ();
@@ -13450,7 +13450,7 @@ remote_target::remote_hostio_open (inferior *inf, const char *filename,
 int
 remote_target::fileio_open (struct inferior *inf, const char *filename,
 			    fileio_open_flags flags, fileio_mode_flags mode,
-			    int warn_if_slow, fileio_error *remote_errno)
+			    bool warn_if_slow, fileio_error *remote_errno)
 {
   return remote_hostio_open (inf, filename, flags, mode, warn_if_slow,
 			     remote_errno);
diff --git a/gdb/target.c b/gdb/target.c
index 611f37c9730..66672f3ca38 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3177,7 +3177,7 @@ fileio_fd_to_fh (target_fd fd)
 int
 target_ops::fileio_open (struct inferior *inf, const char *filename,
 			 fileio_open_flags flags, fileio_mode_flags mode,
-			 int warn_if_slow, fileio_error *target_errno)
+			 bool warn_if_slow, fileio_error *target_errno)
 {
   *target_errno = FILEIO_ENOSYS;
   return -1;
diff --git a/gdb/target.h b/gdb/target.h
index 89f6180b9e5..95434f8e342 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -992,7 +992,7 @@ struct target_ops
        *TARGET_ERRNO).  */
     virtual int fileio_open (struct inferior *inf, const char *filename,
 			     fileio_open_flags flags, fileio_mode_flags mode,
-			     int warn_if_slow, fileio_error *target_errno);
+			     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

-- 
2.53.0

Re: [PATCH v2 1/3] Use a newtype for remote file descriptor

[Simon Marchi]

On 2/27/26 9:28 AM, Tom Tromey wrote:
> 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.

With this patch, we now have 2 different classes names scoped_target_fd.
Is it a problem, ODR-wise?  It looks like they have the same goal, so we
could perhaps merge them.

Simon

Re: [PATCH v2 1/3] Use a newtype for remote file descriptor

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> With this patch, we now have 2 different classes names scoped_target_fd.
Simon> Is it a problem, ODR-wise?  It looks like they have the same goal, so we
Simon> could perhaps merge them.

I didn't notice that.  I'll rename it back.
I've spent too much time on this already.

Tom

Re: [PATCH v2 1/3] Use a newtype for remote file descriptor

[Tom Tromey]

Simon> With this patch, we now have 2 different classes names scoped_target_fd.
Simon> Is it a problem, ODR-wise?  It looks like they have the same goal, so we
Simon> could perhaps merge them.

I looked and they aren't the same, the remote.c one needs the underlying
'int' type.

Tom