From 386c8614d5e65431e977b1b20cc4642f944faca1 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Sat, 23 Sep 2017 11:21:58 -0600 Subject: [PATCH] Remove free_memory_read_result_vector This changes read_memory_robust to return a std::vector, allowing the removal of free_memory_read_result_vector and associated cleanups. This patch also changes the functions it touches to be a bit more robust with regards to deallocation; it's perhaps possible that read_memory_robust could have leaked in some situations. This patch is based on my earlier series to remove some MI cleanups. Regression tested by the buildbot. gdb/ChangeLog 2017-09-29 Tom Tromey * target.c (read_whatever_is_readable): Change type of "result". Update. (free_memory_read_result_vector): Remove. (read_memory_robust): Change return type. Update. * mi/mi-main.c (mi_cmd_data_read_memory_bytes): Update. Use bin2hex, std::string. * target.h (memory_read_result_s): Remove typedef. (free_memory_read_result_vector): Remove. (read_memory_robust): Return std::vector. --- gdb/ChangeLog | 12 ++++++++ gdb/mi/mi-main.c | 40 +++++++------------------- gdb/target.c | 75 ++++++++++++++---------------------------------- gdb/target.h | 35 +++++++++++++--------- 4 files changed, 66 insertions(+), 96 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 005e366488e..ed5d20c565c 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2017-09-29 Tom Tromey + + * target.c (read_whatever_is_readable): Change type of "result". + Update. + (free_memory_read_result_vector): Remove. + (read_memory_robust): Change return type. Update. + * mi/mi-main.c (mi_cmd_data_read_memory_bytes): Update. Use + bin2hex, std::string. + * target.h (memory_read_result_s): Remove typedef. + (free_memory_read_result_vector): Remove. + (read_memory_robust): Return std::vector. + 2017-09-29 Tom Tromey * mi/mi-main.c (captured_mi_execute_command): Use scope_restore. diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 29acf2df0bf..4a61efdc65c 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1506,12 +1506,8 @@ mi_cmd_data_read_memory_bytes (const char *command, char **argv, int argc) { struct gdbarch *gdbarch = get_current_arch (); struct ui_out *uiout = current_uiout; - struct cleanup *cleanups; CORE_ADDR addr; LONGEST length; - memory_read_result_s *read_result; - int ix; - VEC(memory_read_result_s) *result; long offset = 0; int unit_size = gdbarch_addressable_memory_unit_size (gdbarch); int oind = 0; @@ -1548,40 +1544,26 @@ mi_cmd_data_read_memory_bytes (const char *command, char **argv, int argc) addr = parse_and_eval_address (argv[0]) + offset; length = atol (argv[1]); - result = read_memory_robust (current_target.beneath, addr, length); + std::vector result + = read_memory_robust (current_target.beneath, addr, length); - cleanups = make_cleanup (free_memory_read_result_vector, &result); - - if (VEC_length (memory_read_result_s, result) == 0) + if (result.size () == 0) error (_("Unable to read memory.")); ui_out_emit_list list_emitter (uiout, "memory"); - for (ix = 0; - VEC_iterate (memory_read_result_s, result, ix, read_result); - ++ix) + for (const memory_read_result &read_result : result) { ui_out_emit_tuple tuple_emitter (uiout, NULL); - char *data, *p; - int i; - int alloc_len; - - uiout->field_core_addr ("begin", gdbarch, read_result->begin); - uiout->field_core_addr ("offset", gdbarch, read_result->begin - addr); - uiout->field_core_addr ("end", gdbarch, read_result->end); - alloc_len = (read_result->end - read_result->begin) * 2 * unit_size + 1; - data = (char *) xmalloc (alloc_len); + uiout->field_core_addr ("begin", gdbarch, read_result.begin); + uiout->field_core_addr ("offset", gdbarch, read_result.begin - addr); + uiout->field_core_addr ("end", gdbarch, read_result.end); - for (i = 0, p = data; - i < ((read_result->end - read_result->begin) * unit_size); - ++i, p += 2) - { - sprintf (p, "%02x", read_result->data[i]); - } - uiout->field_string ("contents", data); - xfree (data); + std::string data = bin2hex (read_result.data.get (), + (read_result.end - read_result.begin) + * unit_size); + uiout->field_string ("contents", data.c_str ()); } - do_cleanups (cleanups); } /* Implementation of the -data-write_memory command. diff --git a/gdb/target.c b/gdb/target.c index 9abaa581525..aded0ba34f1 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -1630,43 +1630,37 @@ static void read_whatever_is_readable (struct target_ops *ops, const ULONGEST begin, const ULONGEST end, int unit_size, - VEC(memory_read_result_s) **result) + std::vector *result) { - gdb_byte *buf = (gdb_byte *) xmalloc (end - begin); ULONGEST current_begin = begin; ULONGEST current_end = end; int forward; - memory_read_result_s r; ULONGEST xfered_len; /* If we previously failed to read 1 byte, nothing can be done here. */ if (end - begin <= 1) - { - xfree (buf); - return; - } + return; + + gdb::unique_xmalloc_ptr buf ((gdb_byte *) xmalloc (end - begin)); /* Check that either first or the last byte is readable, and give up if not. This heuristic is meant to permit reading accessible memory at the boundary of accessible region. */ if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL, - buf, begin, 1, &xfered_len) == TARGET_XFER_OK) + buf.get (), begin, 1, &xfered_len) == TARGET_XFER_OK) { forward = 1; ++current_begin; } else if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL, - buf + (end - begin) - 1, end - 1, 1, + buf.get () + (end - begin) - 1, end - 1, 1, &xfered_len) == TARGET_XFER_OK) { forward = 0; --current_end; } else - { - xfree (buf); - return; - } + return; /* Loop invariant is that the [current_begin, current_end) was previously found to be not readable as a whole. @@ -1696,7 +1690,7 @@ read_whatever_is_readable (struct target_ops *ops, } xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL, - buf + (first_half_begin - begin) * unit_size, + buf.get () + (first_half_begin - begin) * unit_size, first_half_begin, first_half_end - first_half_begin); @@ -1723,47 +1717,27 @@ read_whatever_is_readable (struct target_ops *ops, if (forward) { /* The [begin, current_begin) range has been read. */ - r.begin = begin; - r.end = current_begin; - r.data = buf; + result->emplace_back (begin, current_end, std::move (buf)); } else { /* The [current_end, end) range has been read. */ LONGEST region_len = end - current_end; - r.data = (gdb_byte *) xmalloc (region_len * unit_size); - memcpy (r.data, buf + (current_end - begin) * unit_size, + gdb::unique_xmalloc_ptr data + ((gdb_byte *) xmalloc (region_len * unit_size)); + memcpy (data.get (), buf.get () + (current_end - begin) * unit_size, region_len * unit_size); - r.begin = current_end; - r.end = end; - xfree (buf); + result->emplace_back (current_end, end, std::move (data)); } - VEC_safe_push(memory_read_result_s, (*result), &r); } -void -free_memory_read_result_vector (void *x) -{ - VEC(memory_read_result_s) **v = (VEC(memory_read_result_s) **) x; - memory_read_result_s *current; - int ix; - - for (ix = 0; VEC_iterate (memory_read_result_s, *v, ix, current); ++ix) - { - xfree (current->data); - } - VEC_free (memory_read_result_s, *v); -} - -VEC(memory_read_result_s) * +std::vector read_memory_robust (struct target_ops *ops, const ULONGEST offset, const LONGEST len) { - VEC(memory_read_result_s) *result = 0; + std::vector result; int unit_size = gdbarch_addressable_memory_unit_size (target_gdbarch ()); - struct cleanup *cleanup = make_cleanup (free_memory_read_result_vector, - &result); LONGEST xfered_total = 0; while (xfered_total < len) @@ -1789,19 +1763,17 @@ read_memory_robust (struct target_ops *ops, else { LONGEST to_read = std::min (len - xfered_total, region_len); - gdb_byte *buffer = (gdb_byte *) xmalloc (to_read * unit_size); - struct cleanup *inner_cleanup = make_cleanup (xfree, buffer); + gdb::unique_xmalloc_ptr buffer + ((gdb_byte *) xmalloc (to_read * unit_size)); LONGEST xfered_partial = - target_read (ops, TARGET_OBJECT_MEMORY, NULL, - (gdb_byte *) buffer, + target_read (ops, TARGET_OBJECT_MEMORY, NULL, buffer.get (), offset + xfered_total, to_read); /* Call an observer, notifying them of the xfer progress? */ if (xfered_partial <= 0) { /* Got an error reading full chunk. See if maybe we can read some subrange. */ - do_cleanups (inner_cleanup); read_whatever_is_readable (ops, offset + xfered_total, offset + xfered_total + to_read, unit_size, &result); @@ -1809,20 +1781,15 @@ read_memory_robust (struct target_ops *ops, } else { - struct memory_read_result r; - - discard_cleanups (inner_cleanup); - r.data = buffer; - r.begin = offset + xfered_total; - r.end = r.begin + xfered_partial; - VEC_safe_push (memory_read_result_s, result, &r); + result.emplace_back (offset + xfered_total, + offset + xfered_total + xfered_partial, + std::move (buffer)); xfered_total += xfered_partial; } QUIT; } } - discard_cleanups (cleanup); return result; } diff --git a/gdb/target.h b/gdb/target.h index 87482dcd91c..f5ad1e5ca8c 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -287,22 +287,31 @@ extern LONGEST target_read (struct target_ops *ops, ULONGEST offset, LONGEST len); struct memory_read_result +{ + memory_read_result (ULONGEST begin_, ULONGEST end_, + gdb::unique_xmalloc_ptr &&data_) + : begin (begin_), + end (end_), + data (std::move (data_)) { - /* First address that was read. */ - ULONGEST begin; - /* Past-the-end address. */ - ULONGEST end; - /* The data. */ - gdb_byte *data; -}; -typedef struct memory_read_result memory_read_result_s; -DEF_VEC_O(memory_read_result_s); + } + + ~memory_read_result () = default; + + memory_read_result (memory_read_result &&other) = default; -extern void free_memory_read_result_vector (void *); + DISABLE_COPY_AND_ASSIGN (memory_read_result); + + /* First address that was read. */ + ULONGEST begin; + /* Past-the-end address. */ + ULONGEST end; + /* The data. */ + gdb::unique_xmalloc_ptr data; +}; -extern VEC(memory_read_result_s)* read_memory_robust (struct target_ops *ops, - const ULONGEST offset, - const LONGEST len); +extern std::vector read_memory_robust + (struct target_ops *ops, const ULONGEST offset, const LONGEST len); /* Request that OPS transfer up to LEN addressable units from BUF to the target's OBJECT. When writing to a memory object, the addressable unit -- 2.30.2