[RFA,10/13] Remove cleanups from linux-tdep.c

Message ID 20171102223612.3642-11-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Nov. 2, 2017, 10:36 p.m. UTC
  This removes some cleanups from linux-tdep.c, replacing them with
def_vector or unique_xmalloc_ptr as appropriate.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* linux-tdep.c (linux_core_info_proc_mappings): Use
	gdb::def_vector.
	(linux_get_siginfo_data): Return gdb::unique_xmalloc_ptr.
	(linux_corefile_thread): Update.
---
 gdb/ChangeLog    |  7 +++++++
 gdb/linux-tdep.c | 45 ++++++++++++++-------------------------------
 2 files changed, 21 insertions(+), 31 deletions(-)
  

Comments

Simon Marchi Nov. 3, 2017, 1:42 a.m. UTC | #1
On 2017-11-02 18:36, Tom Tromey wrote:
> This removes some cleanups from linux-tdep.c, replacing them with
> def_vector or unique_xmalloc_ptr as appropriate.
> 
> gdb/ChangeLog
> 2017-11-02  Tom Tromey  <tom@tromey.com>
> 
> 	* linux-tdep.c (linux_core_info_proc_mappings): Use
> 	gdb::def_vector.
> 	(linux_get_siginfo_data): Return gdb::unique_xmalloc_ptr.
> 	(linux_corefile_thread): Update.

Just to be pedantic, you should mention the change to 
linux_make_mappings_corefile_notes.

> ---
>  gdb/ChangeLog    |  7 +++++++
>  gdb/linux-tdep.c | 45 ++++++++++++++-------------------------------
>  2 files changed, 21 insertions(+), 31 deletions(-)
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 83ff59faee..0350ccea16 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -998,10 +998,9 @@ linux_core_info_proc_mappings (struct gdbarch
> *gdbarch, const char *args)
>  {
>    asection *section;
>    ULONGEST count, page_size;
> -  unsigned char *descdata, *filenames, *descend, *contents;
> +  unsigned char *descdata, *filenames, *descend;
>    size_t note_size;
>    unsigned int addr_size_bits, addr_size;
> -  struct cleanup *cleanup;
>    struct gdbarch *core_gdbarch = gdbarch_from_bfd (core_bfd);
>    /* We assume this for reading 64-bit core files.  */
>    gdb_static_assert (sizeof (ULONGEST) >= 8);
> @@ -1020,12 +1019,12 @@ linux_core_info_proc_mappings (struct gdbarch
> *gdbarch, const char *args)
>    if (note_size < 2 * addr_size)
>      error (_("malformed core note - too short for header"));
> 
> -  contents = (unsigned char *) xmalloc (note_size);
> -  cleanup = make_cleanup (xfree, contents);
> -  if (!bfd_get_section_contents (core_bfd, section, contents, 0, 
> note_size))
> +  gdb::def_vector<unsigned char> contents (note_size);
> +  if (!bfd_get_section_contents (core_bfd, section, contents.data (),
> +				 0, note_size))
>      error (_("could not get core note contents"));
> 
> -  descdata = contents;
> +  descdata = contents.data ();
>    descend = descdata + note_size;
> 
>    if (descdata[note_size - 1] != '\0')
> @@ -1090,8 +1089,6 @@ linux_core_info_proc_mappings (struct gdbarch
> *gdbarch, const char *args)
> 
>        filenames += 1 + strlen ((char *) filenames);
>      }
> -
> -  do_cleanups (cleanup);
>  }
> 
>  /* Implement "info proc" for a corefile.  */
> @@ -1516,7 +1513,6 @@ static char *
>  linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd 
> *obfd,
>  				    char *note_data, int *note_size)
>  {
> -  struct cleanup *cleanup;
>    struct linux_make_mappings_data mapping_data;
>    struct type *long_type
>      = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch), 0, 
> "long");
> @@ -1646,14 +1642,12 @@ linux_collect_thread_registers (const struct
> regcache *regcache,
>     with the size of the data.  The caller is responsible for freeing
>     the data.  */
> 
> -static gdb_byte *
> +static gdb::unique_xmalloc_ptr<gdb_byte>
>  linux_get_siginfo_data (thread_info *thread, struct gdbarch *gdbarch,
>  			LONGEST *size)
>  {
>    struct type *siginfo_type;
> -  gdb_byte *buf;
>    LONGEST bytes_read;
> -  struct cleanup *cleanups;
> 
>    if (!gdbarch_get_siginfo_type_p (gdbarch))
>      return NULL;
> @@ -1663,21 +1657,15 @@ linux_get_siginfo_data (thread_info *thread,
> struct gdbarch *gdbarch,
> 
>    siginfo_type = gdbarch_get_siginfo_type (gdbarch);
> 
> -  buf = (gdb_byte *) xmalloc (TYPE_LENGTH (siginfo_type));
> -  cleanups = make_cleanup (xfree, buf);
> +  gdb::unique_xmalloc_ptr<gdb_byte> buf
> +    ((gdb_byte *) xmalloc (TYPE_LENGTH (siginfo_type)));
> 
>    bytes_read = target_read (&current_target, 
> TARGET_OBJECT_SIGNAL_INFO, NULL,
> -			    buf, 0, TYPE_LENGTH (siginfo_type));
> +			    buf.get (), 0, TYPE_LENGTH (siginfo_type));
>    if (bytes_read == TYPE_LENGTH (siginfo_type))
> -    {
> -      discard_cleanups (cleanups);
> -      *size = bytes_read;
> -    }
> +    *size = bytes_read;
>    else
> -    {
> -      do_cleanups (cleanups);
> -      buf = NULL;
> -    }
> +    return NULL;
> 
>    return buf;

I think this could return a gdb::byte_vector, with an empty vector 
meaning that there is no value, the equivalent of NULL currently (I 
don't think we can have a siginfo type with size 0...).

Simon
  
Tom Tromey Nov. 4, 2017, 4:25 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Just to be pedantic, you should mention the change to
Simon> linux_make_mappings_corefile_notes.

...
Simon> I think this could return a gdb::byte_vector, with an empty vector
Simon> meaning that there is no value, the equivalent of NULL currently (I
Simon> don't think we can have a siginfo type with size 0...).

I made these changes.

Tom
  

Patch

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 83ff59faee..0350ccea16 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -998,10 +998,9 @@  linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
 {
   asection *section;
   ULONGEST count, page_size;
-  unsigned char *descdata, *filenames, *descend, *contents;
+  unsigned char *descdata, *filenames, *descend;
   size_t note_size;
   unsigned int addr_size_bits, addr_size;
-  struct cleanup *cleanup;
   struct gdbarch *core_gdbarch = gdbarch_from_bfd (core_bfd);
   /* We assume this for reading 64-bit core files.  */
   gdb_static_assert (sizeof (ULONGEST) >= 8);
@@ -1020,12 +1019,12 @@  linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
   if (note_size < 2 * addr_size)
     error (_("malformed core note - too short for header"));
 
-  contents = (unsigned char *) xmalloc (note_size);
-  cleanup = make_cleanup (xfree, contents);
-  if (!bfd_get_section_contents (core_bfd, section, contents, 0, note_size))
+  gdb::def_vector<unsigned char> contents (note_size);
+  if (!bfd_get_section_contents (core_bfd, section, contents.data (),
+				 0, note_size))
     error (_("could not get core note contents"));
 
-  descdata = contents;
+  descdata = contents.data ();
   descend = descdata + note_size;
 
   if (descdata[note_size - 1] != '\0')
@@ -1090,8 +1089,6 @@  linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
 
       filenames += 1 + strlen ((char *) filenames);
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Implement "info proc" for a corefile.  */
@@ -1516,7 +1513,6 @@  static char *
 linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
 				    char *note_data, int *note_size)
 {
-  struct cleanup *cleanup;
   struct linux_make_mappings_data mapping_data;
   struct type *long_type
     = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch), 0, "long");
@@ -1646,14 +1642,12 @@  linux_collect_thread_registers (const struct regcache *regcache,
    with the size of the data.  The caller is responsible for freeing
    the data.  */
 
-static gdb_byte *
+static gdb::unique_xmalloc_ptr<gdb_byte>
 linux_get_siginfo_data (thread_info *thread, struct gdbarch *gdbarch,
 			LONGEST *size)
 {
   struct type *siginfo_type;
-  gdb_byte *buf;
   LONGEST bytes_read;
-  struct cleanup *cleanups;
 
   if (!gdbarch_get_siginfo_type_p (gdbarch))
     return NULL;
@@ -1663,21 +1657,15 @@  linux_get_siginfo_data (thread_info *thread, struct gdbarch *gdbarch,
 
   siginfo_type = gdbarch_get_siginfo_type (gdbarch);
 
-  buf = (gdb_byte *) xmalloc (TYPE_LENGTH (siginfo_type));
-  cleanups = make_cleanup (xfree, buf);
+  gdb::unique_xmalloc_ptr<gdb_byte> buf
+    ((gdb_byte *) xmalloc (TYPE_LENGTH (siginfo_type)));
 
   bytes_read = target_read (&current_target, TARGET_OBJECT_SIGNAL_INFO, NULL,
-			    buf, 0, TYPE_LENGTH (siginfo_type));
+			    buf.get (), 0, TYPE_LENGTH (siginfo_type));
   if (bytes_read == TYPE_LENGTH (siginfo_type))
-    {
-      discard_cleanups (cleanups);
-      *size = bytes_read;
-    }
+    *size = bytes_read;
   else
-    {
-      do_cleanups (cleanups);
-      buf = NULL;
-    }
+    return NULL;
 
   return buf;
 }
@@ -1698,17 +1686,14 @@  static void
 linux_corefile_thread (struct thread_info *info,
 		       struct linux_corefile_thread_data *args)
 {
-  struct cleanup *old_chain;
   struct regcache *regcache;
-  gdb_byte *siginfo_data;
   LONGEST siginfo_size = 0;
 
   regcache = get_thread_arch_regcache (info->ptid, args->gdbarch);
 
   target_fetch_registers (regcache, -1);
-  siginfo_data = linux_get_siginfo_data (info, args->gdbarch, &siginfo_size);
-
-  old_chain = make_cleanup (xfree, siginfo_data);
+  gdb::unique_xmalloc_ptr<gdb_byte> siginfo_data
+    = linux_get_siginfo_data (info, args->gdbarch, &siginfo_size);
 
   args->note_data = linux_collect_thread_registers
     (regcache, info->ptid, args->obfd, args->note_data,
@@ -1722,9 +1707,7 @@  linux_corefile_thread (struct thread_info *info,
 					    args->note_data,
 					    args->note_size,
 					    "CORE", NT_SIGINFO,
-					    siginfo_data, siginfo_size);
-
-  do_cleanups (old_chain);
+					    siginfo_data.get (), siginfo_size);
 }
 
 /* Fill the PRPSINFO structure with information about the process being