Re: [patch 4/6] Prepare linux_find_memory_regions_full & co. for move

[Aleksandar Ristovski <aristovski@qnx.com>]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

On 13-03-28 04:28 PM, Jan Kratochvil wrote:
>
> Why you make this change?  That is unrelated to this patchset, it is a recent
> modification by Pedro.
>
> fad14e531       (Pedro Alves    2013-03-11 12:22:16 +0000       3546)  gdb_byte *buffer;
> fad14e531       (Pedro Alves    2013-03-11 12:22:16 +0000       3547)  char *bufstr;
>

...

>
>
> Otherwise OK for this part but it needs a new post, thanks for catching the
> xrealloc.
>
>
> Thanks,
> Jan
>


Ok,

new patch; removed reversal of Pedro's change.


Thanks,

Aleksandar

[-- Attachment #2: 0004-Prepare-linux_find_memory_regions_full-co.-for-move.patch --]
[-- Type: text/x-patch, Size: 12049 bytes --]

From f2cb5b86d02859c11a229b1b20787b0015bc4c68 Mon Sep 17 00:00:00 2001
From: Aleksandar Ristovski <ARistovski@qnx.com>
Date: Wed, 27 Mar 2013 09:49:36 -0400
Subject: [PATCH 4/8] Prepare linux_find_memory_regions_full & co. for move

	* linux-tdep.c (linux_find_memory_region_ftype): Comment.
	(linux_find_memory_regions_full): Change signature and prepare
	for moving to linux-maps.
	(linux_find_memory_regions_data): Rename field 'obfd' to 'data'.
	(linux_find_memory_regions_thunk): New.
	(linux_find_memory_regions_thunk): Use 'data' field instead of 'obfd'.
	(linux_find_memory_regions_gdb): New.
	(linux_find_memory_regions): Rename argument 'obfd' to 'func_data'.
	(linux_make_mappings_corefile_notes): Use
	linux_find_memory_regions_gdb.
	* target.c (target_fileio_read_alloc_1_pread): New function.
	(read_alloc_pread_ftype): New typedef.
	(read_alloc): Refactor from target_fileio_read_alloc_1.
	(target_fileio_read_alloc_1): New implementation. Use read_alloc.
	(read_stralloc_func_ftype): New typedef.
	(read_stralloc): Refactored from target_fileio_read_stralloc.
	(target_fileio_read_stralloc): New implementation, use read_stralloc.
---
 gdb/linux-tdep.c |   97 ++++++++++++++++++++++++++++++++--------------
 gdb/target.c     |  113 +++++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 152 insertions(+), 58 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index a132fc6..c48f4ec 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -661,6 +661,10 @@ linux_core_info_proc (struct gdbarch *gdbarch, char *args,
     error (_("unable to handle request"));
 }
 
+/* Callback function for linux_find_memory_regions_full.  If it returns
+   non-zero linux_find_memory_regions_full returns immediately with that
+   value.  */
+
 typedef int linux_find_memory_region_ftype (ULONGEST vaddr, ULONGEST size,
 					    ULONGEST offset, ULONGEST inode,
 					    int read, int write,
@@ -668,34 +672,40 @@ typedef int linux_find_memory_region_ftype (ULONGEST vaddr, ULONGEST size,
 					    const char *filename,
 					    void *data);
 
-/* List memory regions in the inferior for a corefile.  */
+/* List memory regions in the inferior PID for a corefile.  Call FUNC
+   with FUNC_DATA for each such region.  Return immediately with the
+   value returned by FUNC if it is non-zero.  *MEMORY_TO_FREE_PTR should
+   be registered to be freed automatically if called FUNC throws an
+   exception.  MEMORY_TO_FREE_PTR can be also passed as NULL if it is
+   not used.  Return -1 if error occurs, 0 if all memory regions have
+   been processed or return the value from FUNC if FUNC returns
+   non-zero.  */
 
 static int
-linux_find_memory_regions_full (struct gdbarch *gdbarch,
-				linux_find_memory_region_ftype *func,
-				void *obfd)
+linux_find_memory_regions_full (pid_t pid, linux_find_memory_region_ftype *func,
+				void *func_data, void **memory_to_free_ptr)
 {
   char filename[100];
-  gdb_byte *data;
-
-  /* We need to know the real target PID to access /proc.  */
-  if (current_inferior ()->fake_pid_p)
-    return 1;
+  char *data;
 
-  xsnprintf (filename, sizeof filename,
-	     "/proc/%d/smaps", current_inferior ()->pid);
+  xsnprintf (filename, sizeof filename, "/proc/%d/smaps", (int) pid);
   data = target_fileio_read_stralloc (filename);
   if (data == NULL)
     {
       /* Older Linux kernels did not support /proc/PID/smaps.  */
-      xsnprintf (filename, sizeof filename,
-		 "/proc/%d/maps", current_inferior ()->pid);
+      xsnprintf (filename, sizeof filename, "/proc/%d/maps", (int) pid);
       data = target_fileio_read_stralloc (filename);
     }
   if (data)
     {
-      struct cleanup *cleanup = make_cleanup (xfree, data);
       char *line;
+      int retval = 0;
+
+      if (memory_to_free_ptr != NULL)
+	{
+	  gdb_assert (*memory_to_free_ptr == NULL);
+	  *memory_to_free_ptr = data;
+	}
 
       line = strtok (data, "\n");
       while (line)
@@ -742,15 +752,22 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 	    modified = 1;
 
 	  /* Invoke the callback function to create the corefile segment.  */
-	  func (addr, endaddr - addr, offset, inode,
-		read, write, exec, modified, filename, obfd);
+	  retval = func (addr, endaddr - addr, offset, inode,
+			 read, write, exec, modified, filename, func_data);
+	  if (retval != 0)
+	    break;
 	}
 
-      do_cleanups (cleanup);
-      return 0;
+      if (memory_to_free_ptr != NULL)
+	{
+	  gdb_assert (data == *memory_to_free_ptr);
+	  *memory_to_free_ptr = NULL;
+	}
+      xfree (data);
+      return retval;
     }
 
-  return 1;
+  return -1;
 }
 
 /* A structure for passing information through
@@ -764,9 +781,11 @@ struct linux_find_memory_regions_data
 
   /* The original datum.  */
 
-  void *obfd;
+  void *data;
 };
 
+static linux_find_memory_region_ftype linux_find_memory_regions_thunk;
+
 /* A callback for linux_find_memory_regions that converts between the
    "full"-style callback and find_memory_region_ftype.  */
 
@@ -778,7 +797,30 @@ linux_find_memory_regions_thunk (ULONGEST vaddr, ULONGEST size,
 {
   struct linux_find_memory_regions_data *data = arg;
 
-  return data->func (vaddr, size, read, write, exec, modified, data->obfd);
+  return data->func (vaddr, size, read, write, exec, modified, data->data);
+}
+
+/* Wrapper of linux_find_memory_regions_full handling FAKE_PID_P in GDB.  */
+
+static int
+linux_find_memory_regions_gdb (struct gdbarch *gdbarch,
+			       linux_find_memory_region_ftype *func,
+			       void *func_data)
+{
+  void *memory_to_free = NULL;
+  struct cleanup *cleanup;
+  int retval;
+
+  /* We need to know the real target PID so
+     linux_find_memory_regions_full can access /proc.  */
+  if (current_inferior ()->fake_pid_p)
+    return 1;
+
+  cleanup = make_cleanup (free_current_contents, &memory_to_free);
+  retval = linux_find_memory_regions_full (current_inferior ()->pid,
+					   func, func_data, &memory_to_free);
+  do_cleanups (cleanup);
+  return retval;
 }
 
 /* A variant of linux_find_memory_regions_full that is suitable as the
@@ -786,16 +828,15 @@ linux_find_memory_regions_thunk (ULONGEST vaddr, ULONGEST size,
 
 static int
 linux_find_memory_regions (struct gdbarch *gdbarch,
-			   find_memory_region_ftype func, void *obfd)
+			   find_memory_region_ftype func, void *func_data)
 {
   struct linux_find_memory_regions_data data;
 
   data.func = func;
-  data.obfd = obfd;
+  data.data = func_data;
 
-  return linux_find_memory_regions_full (gdbarch,
-					 linux_find_memory_regions_thunk,
-					 &data);
+  return linux_find_memory_regions_gdb (gdbarch,
+					linux_find_memory_regions_thunk, &data);
 }
 
 /* Determine which signal stopped execution.  */
@@ -977,8 +1018,8 @@ linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
   pack_long (buf, long_type, 1);
   obstack_grow (&data_obstack, buf, TYPE_LENGTH (long_type));
 
-  linux_find_memory_regions_full (gdbarch, linux_make_mappings_callback,
-				  &mapping_data);
+  linux_find_memory_regions_gdb (gdbarch, linux_make_mappings_callback,
+				 &mapping_data);
 
   if (mapping_data.file_count != 0)
     {
diff --git a/gdb/target.c b/gdb/target.c
index 24cc79d..d8ab056 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3476,55 +3476,73 @@ target_fileio_close_cleanup (void *opaque)
   target_fileio_close (fd, &target_errno);
 }
 
+/* Helper for target_fileio_read_alloc_1 to make it interruptible.  */
+
+static int
+target_fileio_read_alloc_1_pread (int handle, gdb_byte *read_buf, int len,
+				  ULONGEST offset, int *target_errno)
+{
+  QUIT;
+
+  return target_fileio_pread (handle, read_buf, len, offset, target_errno);
+}
+
 /* Read target file FILENAME.  Store the result in *BUF_P and
    return the size of the transferred data.  PADDING additional bytes are
    available in *BUF_P.  This is a helper function for
    target_fileio_read_alloc; see the declaration of that function for more
    information.  */
 
+typedef int (read_alloc_pread_ftype) (int handle, gdb_byte *read_buf, int len,
+                                      ULONGEST offset, int *target_errno);
+
 static LONGEST
-target_fileio_read_alloc_1 (const char *filename,
-			    gdb_byte **buf_p, int padding)
+read_alloc (gdb_byte **buf_p, int handle, read_alloc_pread_ftype *pread_func,
+	    int padding, void **memory_to_free_ptr)
 {
-  struct cleanup *close_cleanup;
   size_t buf_alloc, buf_pos;
   gdb_byte *buf;
   LONGEST n;
-  int fd;
   int target_errno;
 
-  fd = target_fileio_open (filename, FILEIO_O_RDONLY, 0700, &target_errno);
-  if (fd == -1)
-    return -1;
-
-  close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd);
-
   /* Start by reading up to 4K at a time.  The target will throttle
      this number down if necessary.  */
   buf_alloc = 4096;
   buf = xmalloc (buf_alloc);
+  if (memory_to_free_ptr != NULL)
+    {
+      gdb_assert (*memory_to_free_ptr == NULL);
+      *memory_to_free_ptr = buf;
+    }
   buf_pos = 0;
   while (1)
     {
-      n = target_fileio_pread (fd, &buf[buf_pos],
-			       buf_alloc - buf_pos - padding, buf_pos,
-			       &target_errno);
-      if (n < 0)
-	{
-	  /* An error occurred.  */
-	  do_cleanups (close_cleanup);
-	  xfree (buf);
-	  return -1;
-	}
-      else if (n == 0)
+      n = pread_func (handle, &buf[buf_pos], buf_alloc - buf_pos - padding,
+		      buf_pos, &target_errno);
+      if (n <= 0)
 	{
-	  /* Read all there was.  */
-	  do_cleanups (close_cleanup);
-	  if (buf_pos == 0)
-	    xfree (buf);
+	  if (n < 0 || (n == 0 && buf_pos == 0))
+	    {
+	      if (memory_to_free_ptr != NULL)
+		{
+		  gdb_assert (buf == *memory_to_free_ptr);
+		  *memory_to_free_ptr = NULL;
+		}
+	      xfree (buf);
+	    }
 	  else
 	    *buf_p = buf;
-	  return buf_pos;
+
+	  if (n < 0)
+	    {
+	      /* An error occurred.  */
+	      return -1;
+	    }
+	  else
+	    {
+	      /* Read all there was.  */
+	      return buf_pos;
+	    }
 	}
 
       buf_pos += n;
@@ -3534,10 +3552,37 @@ target_fileio_read_alloc_1 (const char *filename,
 	{
 	  buf_alloc *= 2;
 	  buf = xrealloc (buf, buf_alloc);
+	  if (memory_to_free_ptr != NULL)
+	    *memory_to_free_ptr = buf;
 	}
+    }
+}
 
-      QUIT;
+static LONGEST
+target_fileio_read_alloc_1 (const char *filename,
+			    gdb_byte **buf_p, int padding)
+{
+  struct cleanup *close_cleanup;
+  int fd, target_errno;
+  void *memory_to_free = NULL;
+  LONGEST retval;
+
+  fd = target_fileio_open (filename, FILEIO_O_RDONLY, 0700, &target_errno);
+  if (fd == -1)
+    return -1;
+
+  close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd);
+
+  make_cleanup (free_current_contents, &memory_to_free);
+  retval = read_alloc (buf_p, fd, target_fileio_read_alloc_1_pread, padding,
+		       &memory_to_free);
+  if (retval >= 0)
+    {
+      /* Returned allocated memory is interesting for the caller.  */
+      memory_to_free = NULL;
     }
+  do_cleanups (close_cleanup);
+  return retval;
 }
 
 /* Read target file FILENAME.  Store the result in *BUF_P and return
@@ -3556,14 +3601,17 @@ target_fileio_read_alloc (const char *filename, gdb_byte **buf_p)
    are returned as allocated but empty strings.  A warning is issued
    if the result contains any embedded NUL bytes.  */
 
-char *
-target_fileio_read_stralloc (const char *filename)
+typedef LONGEST (read_stralloc_func_ftype) (const char *filename,
+					    gdb_byte **buf_p, int padding);
+
+static char *
+read_stralloc (const char *filename, read_stralloc_func_ftype *func)
 {
   gdb_byte *buffer;
   char *bufstr;
   LONGEST i, transferred;
 
-  transferred = target_fileio_read_alloc_1 (filename, &buffer, 1);
+  transferred = func (filename, &buffer, 1);
   bufstr = (char *) buffer;
 
   if (transferred < 0)
@@ -3587,6 +3635,11 @@ target_fileio_read_stralloc (const char *filename)
   return bufstr;
 }
 
+char *
+target_fileio_read_stralloc (const char *filename)
+{
+  return read_stralloc (filename, target_fileio_read_alloc_1);
+}
 
 static int
 default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
-- 
1.7.10.4