diff mbox series

[4/5,libbacktrace] Simplify memory management in build_address_map

Message ID 20181128231741.GA4032@delia
State New
Headers show
Series [1/5,libbacktrace] Factor out backtrace_vector_free | expand

Commit Message

Tom de Vries Nov. 28, 2018, 11:17 p.m. UTC 
Hi,

In the main loop in build_address_map, we first read the abbrevs into a local
variable abbrevs, and then allocate the corresponding unit, after which we assign
the abbrevs to the unit.  This results in dedicated free-upon-failure
handling for the variable, and extra code to make sure that free-upon-failure
doesn't trigger once the unit has taken ownership of the abbrevs.

Simplify this by reversing the order of abbrev reading and unit allocation,
and eliminating the abbrevs local variable.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

[libbacktrace] Simplify memory management in build_address_map

2018-11-28  Tom de Vries  <tdevries@suse.de>

	* dwarf.c (build_address_map): Simplify by removing local variable
	abbrevs.

---
 libbacktrace/dwarf.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

Comments

Li, Pan2 via Gcc-patches Dec. 27, 2018, 4:37 p.m. UTC | #1 
On Wed, Nov 28, 2018 at 3:17 PM Tom de Vries <tdevries@suse.de> wrote:
>
> In the main loop in build_address_map, we first read the abbrevs into a local
> variable abbrevs, and then allocate the corresponding unit, after which we assign
> the abbrevs to the unit.  This results in dedicated free-upon-failure
> handling for the variable, and extra code to make sure that free-upon-failure
> doesn't trigger once the unit has taken ownership of the abbrevs.
>
> Simplify this by reversing the order of abbrev reading and unit allocation,
> and eliminating the abbrevs local variable.
>
> Bootstrapped and reg-tested on x86_64.
>
> OK for trunk?
>
> Thanks,
> - Tom
>
> [libbacktrace] Simplify memory management in build_address_map
>
> 2018-11-28  Tom de Vries  <tdevries@suse.de>
>
>         * dwarf.c (build_address_map): Simplify by removing local variable
>         abbrevs.

This is OK.

Thanks.

Ian
diff mbox series

Patch

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 8a7e94111ac..f843fab7529 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -1432,7 +1432,6 @@  build_address_map (struct backtrace_state *state, uintptr_t base_address,
 		   void *data, struct unit_addrs_vector *addrs)
 {
   struct dwarf_buf info;
-  struct abbrevs abbrevs;
   struct backtrace_vector units;
   size_t units_count;
   size_t i;
@@ -1457,7 +1456,6 @@  build_address_map (struct backtrace_state *state, uintptr_t base_address,
   memset (&units, 0, sizeof units);
   units_count = 0;
 
-  memset (&abbrevs, 0, sizeof abbrevs);
   while (info.left > 0)
     {
       const unsigned char *unit_data_start;
@@ -1488,13 +1486,6 @@  build_address_map (struct backtrace_state *state, uintptr_t base_address,
 	  goto fail;
 	}
 
-      abbrev_offset = read_offset (&unit_buf, is_dwarf64);
-      if (!read_abbrevs (state, abbrev_offset, dwarf_abbrev, dwarf_abbrev_size,
-			 is_bigendian, error_callback, data, &abbrevs))
-	goto fail;
-
-      addrsize = read_byte (&unit_buf);
-
       pu = ((struct unit **)
 	    backtrace_vector_grow (state, sizeof (struct unit *),
 				   error_callback, data, &units));
@@ -1509,6 +1500,14 @@  build_address_map (struct backtrace_state *state, uintptr_t base_address,
       *pu = u;
       ++units_count;
 
+      memset (&u->abbrevs, 0, sizeof u->abbrevs);
+      abbrev_offset = read_offset (&unit_buf, is_dwarf64);
+      if (!read_abbrevs (state, abbrev_offset, dwarf_abbrev, dwarf_abbrev_size,
+			 is_bigendian, error_callback, data, &u->abbrevs))
+	goto fail;
+
+      addrsize = read_byte (&unit_buf);
+
       u->unit_data = unit_buf.buf;
       u->unit_data_len = unit_buf.left;
       u->unit_data_offset = unit_buf.buf - unit_data_start;
@@ -1519,8 +1518,6 @@  build_address_map (struct backtrace_state *state, uintptr_t base_address,
       u->comp_dir = NULL;
       u->abs_filename = NULL;
       u->lineoff = 0;
-      u->abbrevs = abbrevs;
-      memset (&abbrevs, 0, sizeof abbrevs);
 
       /* The actual line number mappings will be read as needed.  */
       u->lines = NULL;
@@ -1559,7 +1556,6 @@  build_address_map (struct backtrace_state *state, uintptr_t base_address,
 	}
       backtrace_vector_free (state, &units, error_callback, data);
     }
-  free_abbrevs (state, &abbrevs, error_callback, data);
   if (addrs->count > 0)
     {
       backtrace_vector_free (state, &addrs->vec, error_callback, data);