[PATCH] Encapsulate target_fileio file descriptors

[Gary Benson <gbenson at redhat dot com>]

Hi all,

Various target_fileio_* functions use integer file descriptors to
refer to open files, which can cause problems if the target stack
changes between the file being opened and other file operations being
performed on that file.  This commit makes the target_fileio_*
functions that use file descriptors use an opaque target_fileio_t
handle instead that encapsulates both the file descriptor itself and
the target_ops vector that should be used to access it.

Built and regtested on RHEL 6.6 x86_64.

Ok to commit?

Thanks,
Gary

---
gdb/ChangeLog:

	* target.h (target_fileio_t): New typedef.
	(target_fileio_open): Changed signature, updated comment.
	(target_fileio_pread): Likewise.
	(target_fileio_pwrite): Likewise.
	(target_fileio_close): Likewise.
	* target.c (target_fileio_handle): New structure.
	(target_fileio_open): Updated to return a target_fileio_t
	instead of an integer file descriptor.  Updated targetdebug
	message to include the target shortname.  Updated comment.
	(target_fileio_pread): Take a target_fileio_t handle as the
	first argument instead of an integer file descriptor.  Updated
	targetdebug message to include the target shortname.  Updated
	comment.
	(target_fileio_pwrite): Likewise.
	(target_fileio_close): Likewise.
	(target_fileio_unlink): Updated comment.
	(target_fileio_readlink): Likewise.
	(target_fileio_close_cleanup): Take a target_fileio_t handle
	as the first argument instead of a pointer to an integer file
	descriptor.  Updated caller.
	(target_fileio_read_alloc_1): Use a target_fileio_t handle
	instead of an integer file descriptor.
---
 gdb/ChangeLog |   25 +++++++++
 gdb/target.c  |  164 ++++++++++++++++++++++++++------------------------------
 gdb/target.h  |   28 ++++++----
 3 files changed, 118 insertions(+), 99 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index af94f48..7e04679 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2684,10 +2684,20 @@ default_fileio_target (void)
     return find_default_run_target ("file I/O");
 }
 
-/* Open FILENAME on the target, using FLAGS and MODE.  Return a
-   target file descriptor, or -1 if an error occurs (and set
-   *TARGET_ERRNO).  */
-int
+/* File handle for target file operations.  */
+
+struct target_fileio_handle
+{
+  /* The target on which this file is open.  */
+  struct target_ops *t;
+
+  /* The file's descriptor on the target.  */
+  int fd;
+};
+
+/* See target.h.  */
+
+target_fileio_t
 target_fileio_open (const char *filename, int flags, int mode,
 		    int *target_errno)
 {
@@ -2698,107 +2708,86 @@ target_fileio_open (const char *filename, int flags, int mode,
       if (t->to_fileio_open != NULL)
 	{
 	  int fd = t->to_fileio_open (t, filename, flags, mode, target_errno);
+	  target_fileio_t fh = NULL;
 
 	  if (targetdebug)
 	    fprintf_unfiltered (gdb_stdlog,
-				"target_fileio_open (%s,0x%x,0%o) = %d (%d)\n",
-				filename, flags, mode,
+				"target_fileio_open (%s,0x%x,0%o)"
+				" = %s:%d (%d)\n",
+				filename, flags, mode, t->to_shortname,
 				fd, fd != -1 ? 0 : *target_errno);
-	  return fd;
+	  if (fd >= 0)
+	    {
+	      fh = XCNEW (struct target_fileio_handle);
+	      fh->t = t;
+	      fh->fd = fd;
+	    }
+
+	  return fh;
 	}
     }
 
   *target_errno = FILEIO_ENOSYS;
-  return -1;
+  return NULL;
 }
 
-/* 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).  */
+/* See target.h.  */
+
 int
-target_fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
-		      ULONGEST offset, int *target_errno)
+target_fileio_pwrite (target_fileio_t fh, const gdb_byte *write_buf,
+		      int len, ULONGEST offset, int *target_errno)
 {
-  struct target_ops *t;
+  int ret = fh->t->to_fileio_pwrite (fh->t, fh->fd, write_buf, len,
+				     offset, target_errno);
 
-  for (t = default_fileio_target (); t != NULL; t = t->beneath)
-    {
-      if (t->to_fileio_pwrite != NULL)
-	{
-	  int ret = t->to_fileio_pwrite (t, fd, write_buf, len, offset,
-					 target_errno);
-
-	  if (targetdebug)
-	    fprintf_unfiltered (gdb_stdlog,
-				"target_fileio_pwrite (%d,...,%d,%s) "
-				"= %d (%d)\n",
-				fd, len, pulongest (offset),
-				ret, ret != -1 ? 0 : *target_errno);
-	  return ret;
-	}
-    }
-
-  *target_errno = FILEIO_ENOSYS;
-  return -1;
+  if (targetdebug)
+    fprintf_unfiltered (gdb_stdlog,
+			"target_fileio_pwrite (%s:%d,...,%d,%s)"
+			" = %d (%d)\n",
+			fh->t->to_shortname, fh->fd, len,
+			pulongest (offset), ret,
+			ret != -1 ? 0 : *target_errno);
+  return ret;
 }
 
-/* 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).  */
+/* See target.h.  */
+
 int
-target_fileio_pread (int fd, gdb_byte *read_buf, int len,
+target_fileio_pread (target_fileio_t fh, gdb_byte *read_buf, int len,
 		     ULONGEST offset, int *target_errno)
 {
-  struct target_ops *t;
+  int ret = fh->t->to_fileio_pread (fh->t, fh->fd, read_buf, len,
+				    offset, target_errno);
 
-  for (t = default_fileio_target (); t != NULL; t = t->beneath)
-    {
-      if (t->to_fileio_pread != NULL)
-	{
-	  int ret = t->to_fileio_pread (t, fd, read_buf, len, offset,
-					target_errno);
-
-	  if (targetdebug)
-	    fprintf_unfiltered (gdb_stdlog,
-				"target_fileio_pread (%d,...,%d,%s) "
-				"= %d (%d)\n",
-				fd, len, pulongest (offset),
-				ret, ret != -1 ? 0 : *target_errno);
-	  return ret;
-	}
-    }
-
-  *target_errno = FILEIO_ENOSYS;
-  return -1;
+  if (targetdebug)
+    fprintf_unfiltered (gdb_stdlog,
+			"target_fileio_pread (%s:%d,...,%d,%s)"
+			" = %d (%d)\n",
+			fh->t->to_shortname, fh->fd, len,
+			pulongest (offset), ret,
+			ret != -1 ? 0 : *target_errno);
+  return ret;
 }
 
-/* Close FD on the target.  Return 0, or -1 if an error occurs
-   (and set *TARGET_ERRNO).  */
+/* See target.h.  */
+
 int
-target_fileio_close (int fd, int *target_errno)
+target_fileio_close (target_fileio_t fh, int *target_errno)
 {
-  struct target_ops *t;
+  int ret = fh->t->to_fileio_close (fh->t, fh->fd, target_errno);
 
-  for (t = default_fileio_target (); t != NULL; t = t->beneath)
-    {
-      if (t->to_fileio_close != NULL)
-	{
-	  int ret = t->to_fileio_close (t, fd, target_errno);
-
-	  if (targetdebug)
-	    fprintf_unfiltered (gdb_stdlog,
-				"target_fileio_close (%d) = %d (%d)\n",
-				fd, ret, ret != -1 ? 0 : *target_errno);
-	  return ret;
-	}
-    }
+  if (targetdebug)
+    fprintf_unfiltered (gdb_stdlog,
+			"target_fileio_close (%s:%d) = %d (%d)\n",
+			fh->t->to_shortname, fh->fd, ret,
+			ret != -1 ? 0 : *target_errno);
+  xfree (fh);
 
-  *target_errno = FILEIO_ENOSYS;
-  return -1;
+  return ret;
 }
 
-/* Unlink FILENAME on the target.  Return 0, or -1 if an error
-   occurs (and set *TARGET_ERRNO).  */
+/* See target.h.  */
+
 int
 target_fileio_unlink (const char *filename, int *target_errno)
 {
@@ -2822,9 +2811,8 @@ target_fileio_unlink (const char *filename, int *target_errno)
   return -1;
 }
 
-/* Read value of symbolic link FILENAME on the target.  Return a
-   null-terminated string allocated via xmalloc, or NULL if an error
-   occurs (and set *TARGET_ERRNO).  */
+/* See target.h.  */
+
 char *
 target_fileio_readlink (const char *filename, int *target_errno)
 {
@@ -2852,10 +2840,10 @@ target_fileio_readlink (const char *filename, int *target_errno)
 static void
 target_fileio_close_cleanup (void *opaque)
 {
-  int fd = *(int *) opaque;
+  target_fileio_t fh = (target_fileio_t) opaque;
   int target_errno;
 
-  target_fileio_close (fd, &target_errno);
+  target_fileio_close (fh, &target_errno);
 }
 
 /* Read target file FILENAME.  Store the result in *BUF_P and
@@ -2872,14 +2860,14 @@ target_fileio_read_alloc_1 (const char *filename,
   size_t buf_alloc, buf_pos;
   gdb_byte *buf;
   LONGEST n;
-  int fd;
+  target_fileio_t fh;
   int target_errno;
 
-  fd = target_fileio_open (filename, FILEIO_O_RDONLY, 0700, &target_errno);
-  if (fd == -1)
+  fh = target_fileio_open (filename, FILEIO_O_RDONLY, 0700, &target_errno);
+  if (fh == NULL)
     return -1;
 
-  close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd);
+  close_cleanup = make_cleanup (target_fileio_close_cleanup, fh);
 
   /* Start by reading up to 4K at a time.  The target will throttle
      this number down if necessary.  */
@@ -2888,7 +2876,7 @@ target_fileio_read_alloc_1 (const char *filename,
   buf_pos = 0;
   while (1)
     {
-      n = target_fileio_pread (fd, &buf[buf_pos],
+      n = target_fileio_pread (fh, &buf[buf_pos],
 			       buf_alloc - buf_pos - padding, buf_pos,
 			       &target_errno);
       if (n < 0)
diff --git a/gdb/target.h b/gdb/target.h
index c95e1a4..1edee84 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1915,27 +1915,33 @@ extern int target_search_memory (CORE_ADDR start_addr,
 
 /* Target file operations.  */
 
-/* Open FILENAME on the target, using FLAGS and MODE.  Return a
-   target file descriptor, or -1 if an error occurs (and set
-   *TARGET_ERRNO).  */
-extern int target_fileio_open (const char *filename, int flags, int mode,
-			       int *target_errno);
+/* File handle for target file operations.  */
+typedef struct target_fileio_handle *target_fileio_t;
 
-/* Write up to LEN bytes from WRITE_BUF to FD on the target.
+/* Open FILENAME on the target, using FLAGS and MODE.  Return a target
+   file handle, or NULL if an error occurs (and set *TARGET_ERRNO).  */
+extern target_fileio_t target_fileio_open (const char *filename,
+					   int flags, int mode,
+					   int *target_errno);
+
+/* Write up to LEN bytes from WRITE_BUF to FH 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,
+extern int target_fileio_pwrite (target_fileio_t fh,
+				 const gdb_byte *write_buf, int len,
 				 ULONGEST offset, int *target_errno);
 
-/* Read up to LEN bytes FD on the target into READ_BUF.
+/* Read up to LEN bytes from FH 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_fileio_t fh,
+				gdb_byte *read_buf, int len,
 				ULONGEST offset, int *target_errno);
 
-/* Close FD on the target.  Return 0, or -1 if an error occurs
+/* Close FH on the target.  Return 0, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
-extern int target_fileio_close (int fd, int *target_errno);
+extern int target_fileio_close (target_fileio_t fh,
+				int *target_errno);
 
 /* Unlink FILENAME on the target.  Return 0, or -1 if an error
    occurs (and set *TARGET_ERRNO).  */
-- 
1.7.1