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

[PATCH 0/2] 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.

Note I could not test the solib-rocm.c changes.  These are best
effort.

Signed-off-by: Tom Tromey <tromey@adacore.com>
---
Tom Tromey (2):
      Use a newtype for remote file descriptor
      Use enum types for remote fileio flags

 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         | 19 ++++++-----
 gdb/solib-rocm.c     | 30 ++++++++---------
 gdb/sparc64-tdep.c   | 23 ++++++-------
 gdb/target.c         | 76 +++++++++++++++++++++++--------------------
 gdb/target.h         | 32 +++++++++++-------
 gdbserver/hostio.cc  |  4 +--
 gdbsupport/fileio.cc | 20 ++++++------
 gdbsupport/fileio.h  | 81 ++++++++++++++++++++++++++--------------------
 14 files changed, 186 insertions(+), 214 deletions(-)
---
base-commit: adbc0c55421dd5e31bb1903512dfb8f5211ee5b0
change-id: 20260225-target-fd-newtype-b5c3e3ad31b7

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

[PATCH 1/2] 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.

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

[PATCH 2/2] 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.
---
 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/solib-rocm.c     |  2 +-
 gdb/sparc64-tdep.c   |  5 +--
 gdb/target.c         | 13 +++++---
 gdb/target.h         | 10 +++---
 gdbserver/hostio.cc  |  4 +--
 gdbsupport/fileio.cc | 20 ++++++------
 gdbsupport/fileio.h  | 81 ++++++++++++++++++++++++++--------------------
 13 files changed, 111 insertions(+), 150 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 22f584f0c57..e5b6942287f 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/solib-rocm.c b/gdb/solib-rocm.c
index 6e05913696a..651a958f162 100644
--- a/gdb/solib-rocm.c
+++ b/gdb/solib-rocm.c
@@ -82,7 +82,7 @@ rocm_solib_fd_cache::open (const std::string &filename,
       /* The file is not yet opened on the target.  */
       remote_fd fd
 	= target_fileio_open (m_inferior, filename.c_str (), FILEIO_O_RDONLY,
-			      false, 0, target_errno);
+			      0, false, target_errno);
       if (fd != remote_fd::INVALID)
 	m_cache.emplace (std::piecewise_construct,
 			 std::forward_as_tuple (filename),
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 6919a1c37d3..6733aa7cc48 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -287,8 +287,9 @@ 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,
+					  FILEIO_S_IRWXU, 0, &target_errno);
   return proc->stat.tag_fd;
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index cfdf339b013..012ece5dad5 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3176,8 +3176,8 @@ fileio_fd_to_fh (remote_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,
 
 remote_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 ())
     {
@@ -3259,7 +3260,8 @@ 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 (fd),
+				  unsigned (flags), unsigned (mode),
+				  warn_if_slow, int (fd),
 				  int (fd) != -1 ? 0 : *target_errno);
       return result;
     }
@@ -3479,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 () == remote_fd::INVALID)
     return -1;
 
diff --git a/gdb/target.h b/gdb/target.h
index 87cf8acac28..388acb9cff4 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 remote_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 remote_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..04ea99ea22c 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,54 @@
    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_lseek_flag : unsigned
+{
+  FILEIO_SEEK_SET = 0,
+  FILEIO_SEEK_CUR = 1,
+  FILEIO_SEEK_END = 2,
+};
+DEF_ENUM_FLAGS_TYPE (enum fileio_lseek_flag, fileio_lseek_flags);
 
 /* errno values */
 enum fileio_error
@@ -146,12 +159,12 @@ 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.  */
 
-extern int fileio_to_host_openflags (int fflags, int *flags);
+extern int fileio_to_host_openflags (fileio_open_flags fflags, int *flags);
 
 /* Convert File-I/O mode FMODE to host format, storing
    the result in *MODE.  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);
 
 /* 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

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

[Simon Marchi]

On 2/25/26 4:25 PM, 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.
> 
> The solib-rocm.c changes are best-effort, as I can't compile this
> file.

solib-rocm.c just needs one more change to build:

diff --git i/gdb/solib-rocm.c w/gdb/solib-rocm.c
index 651a958f1620..6ebe0c2b8cd2 100644
--- i/gdb/solib-rocm.c
+++ w/gdb/solib-rocm.c
@@ -94,7 +94,7 @@ 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 != remote_fd::INVALID);
       return it->second.fd;
     }
 }

Otherwise, LGTM.  Just a naming nit below.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Btw, if you want to build the ROCm stuff as part of your all-targets
build, it's not that difficult, as long as your distro has some ROCm
packages.  Looks there are some for Fedora (which I believe you use):

https://src.fedoraproject.org/rpms/rocdbgapi

You should only need the dbgapi package.  You configure with
--with-amd-dbgapi, which will make configure fail if it can't find the
dbgapi library.  configure will hopefully find the dbgapi library in its
default installed location, otherwise you have to point PKG_CONFIG_PATH
to it.  That's pretty much it.

> 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;

Calling it remote_fd in the context of ROCm is weird, since it's not
remote.  Perhaps call it target_fd?  It would make sense to me, since
it's the target file I/O interface.

Simon

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

[Simon Marchi]

On 2/25/26 4:25 PM, Tom Tromey wrote:
> @@ -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;

One more nit: in this function, you don't need to case fd to int, since
fd is an int.  You could either remove the cast, or use `result` in the
debug print.

Simon

Re: [PATCH 2/2] Use enum types for remote fileio flags

[Simon Marchi]

On 2/25/26 4:25 PM, Tom Tromey wrote:
> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
> index 6919a1c37d3..6733aa7cc48 100644
> --- a/gdb/sparc64-tdep.c
> +++ b/gdb/sparc64-tdep.c
> @@ -287,8 +287,9 @@ 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,
> +					  FILEIO_S_IRWXU, 0, &target_errno);

Perhaps change that 0 to false (the `bool warn_if_slow` param).

So the flags here goes from "false" (obviously wrong) to FILEIO_S_IRWXU.
Since we don't pass the _CREATE flag, I think that the value for mode
is irrelevant.  If so, it might be clearer to pass 0, otherwise it makes
it seem like the value we pass is meaningful, which can be confusing.

>  /* lseek(2) flags */
> -#define FILEIO_SEEK_SET             0
> -#define FILEIO_SEEK_CUR             1
> -#define FILEIO_SEEK_END             2
> +enum fileio_lseek_flag : unsigned
> +{
> +  FILEIO_SEEK_SET = 0,
> +  FILEIO_SEEK_CUR = 1,
> +  FILEIO_SEEK_END = 2,
> +};
> +DEF_ENUM_FLAGS_TYPE (enum fileio_lseek_flag, fileio_lseek_flags);

I don't think these values are actually flags, theys are all mutually
exclusive, I think it should be a regular enum (or enum class).

If we want to mimic the posix API, you could call it
"fileio_lseek_whence".

But I also noticed the enum type is not actually used.  It could be used
in remote_fileio_seek_flag_to_host, although it wouldn't be terribly
useful.  So perhaps this enum could be anonymous actually.

>  
>  /* errno values */
>  enum fileio_error
> @@ -146,12 +159,12 @@ 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.  */
>  
> -extern int fileio_to_host_openflags (int fflags, int *flags);
> +extern int fileio_to_host_openflags (fileio_open_flags fflags, int *flags);

In the implementation, the second parameter is named open_flags_p.  You
could maybe rename it here.  That would free up "flags" for the first
parameter, if you want.

Simon

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

[Tom Tromey]

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

Simon> solib-rocm.c just needs one more change to build:

Simon> -      gdb_assert (it->second.fd != -1);
Simon> +      gdb_assert (it->second.fd != remote_fd::INVALID);

Thanks.

Simon> Calling it remote_fd in the context of ROCm is weird, since it's not
Simon> remote.  Perhaps call it target_fd?  It would make sense to me, since
Simon> it's the target file I/O interface.

I renamed it for v2.

Tom

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

[Tom Tromey]

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

>> 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;

Simon> One more nit: in this function, you don't need to case fd to int, since
Simon> fd is an int.  You could either remove the cast, or use `result` in the
Simon> debug print.

Yeah, I meant to use result there, I've fixed this.

Tom

Re: [PATCH 2/2] Use enum types for remote fileio flags

[Tom Tromey]

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

>> +  proc->stat.tag_fd = target_fileio_open (NULL, cl_name,
>> +					  FILEIO_O_RDWR | FILEIO_O_EXCL,
>> +					  FILEIO_S_IRWXU, 0, &target_errno);

Simon> Perhaps change that 0 to false (the `bool warn_if_slow` param).

I added a patch to fix up the warn_if_slow stuff.

Simon> So the flags here goes from "false" (obviously wrong) to FILEIO_S_IRWXU.
Simon> Since we don't pass the _CREATE flag, I think that the value for mode
Simon> is irrelevant.  If so, it might be clearer to pass 0, otherwise it makes
Simon> it seem like the value we pass is meaningful, which can be confusing.

I fixed this.

Simon> But I also noticed the enum type is not actually used.  It could be used
Simon> in remote_fileio_seek_flag_to_host, although it wouldn't be terribly
Simon> useful.  So perhaps this enum could be anonymous actually.

Yeah I did this, thanks.

>> 
>> /* errno values */
>> enum fileio_error
>> @@ -146,12 +159,12 @@ 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.  */
>> 
>> -extern int fileio_to_host_openflags (int fflags, int *flags);
>> +extern int fileio_to_host_openflags (fileio_open_flags fflags, int *flags);

Simon> In the implementation, the second parameter is named open_flags_p.  You
Simon> could maybe rename it here.  That would free up "flags" for the first
Simon> parameter, if you want.

I made them use the same names.  I didn't change it to 'flags' though.

Tom