From patchwork Sun Sep 24 02:11:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 23114 Received: (qmail 16446 invoked by alias); 24 Sep 2017 02:12:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 125963 invoked by uid 89); 24 Sep 2017 02:12:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=Got, QUIT, Request, observer X-HELO: gproxy9-pub.mail.unifiedlayer.com Received: from gproxy9-pub.mail.unifiedlayer.com (HELO gproxy9-pub.mail.unifiedlayer.com) (69.89.20.122) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 24 Sep 2017 02:12:09 +0000 Received: from cmgw4 (unknown [10.0.90.85]) by gproxy9.mail.unifiedlayer.com (Postfix) with ESMTP id 1B9581E0ABE for ; Sat, 23 Sep 2017 20:12:07 -0600 (MDT) Received: from box522.bluehost.com ([74.220.219.122]) by cmgw4 with id DEC31w03J2f2jeq01EC6Zb; Sat, 23 Sep 2017 20:12:07 -0600 X-Authority-Analysis: v=2.2 cv=OZLoNlbY c=1 sm=1 tr=0 a=GsOEXm/OWkKvwdLVJsfwcA==:117 a=GsOEXm/OWkKvwdLVJsfwcA==:17 a=2JCJgTwv5E4A:10 a=zstS-IiYAAAA:8 a=kzPt6fcYsjM1BjzsKV8A:9 a=-h7hWphtx5gjGbNG:21 a=i7kaRwjww-Ku4ykf:21 a=4G6NA9xxw8l3yy4pmD5M:22 Received: from 75-166-76-94.hlrn.qwest.net ([75.166.76.94]:55904 helo=bapiya.Home) by box522.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.87) (envelope-from ) id 1dvwOp-001P3n-BG; Sat, 23 Sep 2017 20:12:03 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA] Remove free_memory_read_result_vector Date: Sat, 23 Sep 2017 20:11:59 -0600 Message-Id: <20170924021159.21353-1-tom@tromey.com> X-BWhitelist: no X-Exim-ID: 1dvwOp-001P3n-BG X-Source-Sender: 75-166-76-94.hlrn.qwest.net (bapiya.Home) [75.166.76.94]:55904 X-Source-Auth: tom+tromey.com X-Email-Count: 1 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTIyLmJsdWVob3N0LmNvbQ== X-Local-Domain: yes 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. ChangeLog 2017-09-23 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 | 40 ++++++++++++++++++++---------- 4 files changed, 71 insertions(+), 96 deletions(-) diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 3362597..b1f2378 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1501,12 +1501,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; @@ -1543,40 +1539,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 b1e6011..17fd0f1 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 87482dc..e089527 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -287,22 +287,36 @@ 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); + } -extern void free_memory_read_result_vector (void *); + ~memory_read_result () = default; + + memory_read_result (memory_read_result &&other) + : begin (other.begin), + end (other.end), + data (std::move (other.data)) + { + } + + 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