This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFA] Change target_write_memory_blocks to use std::vector
- From: Tom Tromey <tom at tromey dot com>
- To: gdb-patches at sourceware dot org
- Cc: Tom Tromey <tom at tromey dot com>
- Date: Sun, 25 Feb 2018 10:37:03 -0700
- Subject: [RFA] Change target_write_memory_blocks to use std::vector
- Authentication-results: sourceware.org; auth=none
This changes target_write_memory_blocks to use std::vector, rather
than VEC. This allows the removal of some cleanups.
Regression tested by the buildbot, though I am not certain that this
code path is tested there, so extra care should be taken during
review.
ChangeLog
2018-02-25 Tom Tromey <tom@tromey.com>
* target.h (memory_write_request_s): Remove typedef. Don't define
VEC.
(target_write_memory_blocks): Change argument to std::vector.
* target-memory.c (compare_block_starting_address): Return bool.
Change argument types.
(claim_memory): Change arguments to use std::vector.
(split_regular_and_flash_blocks, blocks_to_erase)
(compute_garbled_blocks): Likewise.
(cleanup_request_data, cleanup_write_requests_vector): Remove.
(target_write_memory_blocks): Change argument to std::vector.
* symfile.c (struct load_section_data): Add constructor and
destructor. Use std::vector for "requests".
(load_section_callback): Update.
(clear_memory_write_data): Remove.
(generic_load): Update.
---
gdb/ChangeLog | 18 ++++
gdb/symfile.c | 66 ++++++---------
gdb/target-memory.c | 236 ++++++++++++++++++++--------------------------------
gdb/target.h | 9 +-
4 files changed, 140 insertions(+), 189 deletions(-)
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 699d9e6fe0..104d149584 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1894,9 +1894,22 @@ add_section_size_callback (bfd *abfd, asection *asec, void *data)
/* Opaque data for load_section_callback. */
struct load_section_data {
- CORE_ADDR load_offset;
- struct load_progress_data *progress_data;
- VEC(memory_write_request_s) *requests;
+ CORE_ADDR load_offset = 0;
+ struct load_progress_data *progress_data = nullptr;
+ std::vector<struct memory_write_request> requests;
+
+ load_section_data ()
+ {
+ }
+
+ ~load_section_data ()
+ {
+ for (auto &&request : requests)
+ {
+ xfree (request.data);
+ xfree (request.baton);
+ }
+ }
};
/* Opaque data for load_progress. */
@@ -1988,7 +2001,7 @@ load_progress (ULONGEST bytes, void *untyped_arg)
static void
load_section_callback (bfd *abfd, asection *asec, void *data)
{
- struct memory_write_request *new_request;
+ struct memory_write_request new_request;
struct load_section_data *args = (struct load_section_data *) data;
struct load_progress_section_data *section_data;
bfd_size_type size = bfd_get_section_size (asec);
@@ -2001,44 +2014,25 @@ load_section_callback (bfd *abfd, asection *asec, void *data)
if (size == 0)
return;
- new_request = VEC_safe_push (memory_write_request_s,
- args->requests, NULL);
- memset (new_request, 0, sizeof (struct memory_write_request));
+ memset (&new_request, 0, sizeof (struct memory_write_request));
section_data = XCNEW (struct load_progress_section_data);
- new_request->begin = bfd_section_lma (abfd, asec) + args->load_offset;
- new_request->end = new_request->begin + size; /* FIXME Should size
- be in instead? */
- new_request->data = (gdb_byte *) xmalloc (size);
- new_request->baton = section_data;
+ new_request.begin = bfd_section_lma (abfd, asec) + args->load_offset;
+ new_request.end = new_request.begin + size; /* FIXME Should size
+ be in instead? */
+ new_request.data = (gdb_byte *) xmalloc (size);
+ new_request.baton = section_data;
- buffer = new_request->data;
+ buffer = new_request.data;
section_data->cumulative = args->progress_data;
section_data->section_name = sect_name;
section_data->section_size = size;
- section_data->lma = new_request->begin;
+ section_data->lma = new_request.begin;
section_data->buffer = buffer;
- bfd_get_section_contents (abfd, asec, buffer, 0, size);
-}
-
-/* Clean up an entire memory request vector, including load
- data and progress records. */
-
-static void
-clear_memory_write_data (void *arg)
-{
- VEC(memory_write_request_s) **vec_p = (VEC(memory_write_request_s) **) arg;
- VEC(memory_write_request_s) *vec = *vec_p;
- int i;
- struct memory_write_request *mr;
+ args->requests.push_back (new_request);
- for (i = 0; VEC_iterate (memory_write_request_s, vec, i, mr); ++i)
- {
- xfree (mr->data);
- xfree (mr->baton);
- }
- VEC_free (memory_write_request_s, vec);
+ bfd_get_section_contents (abfd, asec, buffer, 0, size);
}
static void print_transfer_performance (struct ui_file *stream,
@@ -2049,19 +2043,15 @@ static void print_transfer_performance (struct ui_file *stream,
void
generic_load (const char *args, int from_tty)
{
- struct cleanup *old_cleanups;
struct load_section_data cbdata;
struct load_progress_data total_progress;
struct ui_out *uiout = current_uiout;
CORE_ADDR entry;
- memset (&cbdata, 0, sizeof (cbdata));
memset (&total_progress, 0, sizeof (total_progress));
cbdata.progress_data = &total_progress;
- old_cleanups = make_cleanup (clear_memory_write_data, &cbdata.requests);
-
if (args == NULL)
error_no_arg (_("file to load"));
@@ -2132,8 +2122,6 @@ generic_load (const char *args, int from_tty)
print_transfer_performance (gdb_stdout, total_progress.data_count,
total_progress.write_count,
end_time - start_time);
-
- do_cleanups (old_cleanups);
}
/* Report on STREAM the performance of a memory transfer operation,
diff --git a/gdb/target-memory.c b/gdb/target-memory.c
index 81faa22a60..a945983723 100644
--- a/gdb/target-memory.c
+++ b/gdb/target-memory.c
@@ -26,20 +26,11 @@
#include "gdb_sys_time.h"
#include <algorithm>
-static int
-compare_block_starting_address (const void *a, const void *b)
+static bool
+compare_block_starting_address (const memory_write_request &a_req,
+ const memory_write_request &b_req)
{
- const struct memory_write_request *a_req
- = (const struct memory_write_request *) a;
- const struct memory_write_request *b_req
- = (const struct memory_write_request *) b;
-
- if (a_req->begin < b_req->begin)
- return -1;
- else if (a_req->begin == b_req->begin)
- return 0;
- else
- return 1;
+ return a_req.begin < b_req.begin;
}
/* Adds to RESULT all memory write requests from BLOCK that are
@@ -49,17 +40,15 @@ compare_block_starting_address (const void *a, const void *b)
that part of the memory request will be added. */
static void
-claim_memory (VEC(memory_write_request_s) *blocks,
- VEC(memory_write_request_s) **result,
+claim_memory (const std::vector<memory_write_request> &blocks,
+ std::vector<memory_write_request> *result,
ULONGEST begin,
ULONGEST end)
{
- int i;
ULONGEST claimed_begin;
ULONGEST claimed_end;
- struct memory_write_request *r;
- for (i = 0; VEC_iterate (memory_write_request_s, blocks, i, r); ++i)
+ for (const memory_write_request &r : blocks)
{
/* If the request doesn't overlap [BEGIN, END), skip it. We
must handle END == 0 meaning the top of memory; we don't yet
@@ -67,28 +56,28 @@ claim_memory (VEC(memory_write_request_s) *blocks,
memory, but there's an assertion in
target_write_memory_blocks which checks for that. */
- if (begin >= r->end)
+ if (begin >= r.end)
continue;
- if (end != 0 && end <= r->begin)
+ if (end != 0 && end <= r.begin)
continue;
- claimed_begin = std::max (begin, r->begin);
+ claimed_begin = std::max (begin, r.begin);
if (end == 0)
- claimed_end = r->end;
+ claimed_end = r.end;
else
- claimed_end = std::min (end, r->end);
+ claimed_end = std::min (end, r.end);
- if (claimed_begin == r->begin && claimed_end == r->end)
- VEC_safe_push (memory_write_request_s, *result, r);
+ if (claimed_begin == r.begin && claimed_end == r.end)
+ result->push_back (r);
else
{
- struct memory_write_request *n =
- VEC_safe_push (memory_write_request_s, *result, NULL);
+ struct memory_write_request n = r;
+
+ n.begin = claimed_begin;
+ n.end = claimed_end;
+ n.data += claimed_begin - r.begin;
- *n = *r;
- n->begin = claimed_begin;
- n->end = claimed_end;
- n->data += claimed_begin - r->begin;
+ result->push_back (n);
}
}
}
@@ -98,9 +87,9 @@ claim_memory (VEC(memory_write_request_s) *blocks,
regular memory to REGULAR_BLOCKS. */
static void
-split_regular_and_flash_blocks (VEC(memory_write_request_s) *blocks,
- VEC(memory_write_request_s) **regular_blocks,
- VEC(memory_write_request_s) **flash_blocks)
+split_regular_and_flash_blocks (const std::vector<memory_write_request> &blocks,
+ std::vector<memory_write_request> *regular_blocks,
+ std::vector<memory_write_request> *flash_blocks)
{
struct mem_region *region;
CORE_ADDR cur_address;
@@ -116,7 +105,7 @@ split_regular_and_flash_blocks (VEC(memory_write_request_s) *blocks,
cur_address = 0;
while (1)
{
- VEC(memory_write_request_s) **r;
+ std::vector<memory_write_request> *r;
region = lookup_mem_region (cur_address);
r = region->attrib.mode == MEM_FLASH ? flash_blocks : regular_blocks;
@@ -156,34 +145,29 @@ block_boundaries (CORE_ADDR address, CORE_ADDR *begin, CORE_ADDR *end)
returns write requests covering each group of flash blocks which must
be erased. */
-static VEC(memory_write_request_s) *
-blocks_to_erase (VEC(memory_write_request_s) *written)
+static std::vector<memory_write_request>
+blocks_to_erase (const std::vector<memory_write_request> &written)
{
- unsigned i;
- struct memory_write_request *ptr;
-
- VEC(memory_write_request_s) *result = NULL;
+ std::vector<memory_write_request> result;
- for (i = 0; VEC_iterate (memory_write_request_s, written, i, ptr); ++i)
+ for (const memory_write_request &request : written)
{
CORE_ADDR begin, end;
- block_boundaries (ptr->begin, &begin, 0);
- block_boundaries (ptr->end - 1, 0, &end);
+ block_boundaries (request.begin, &begin, 0);
+ block_boundaries (request.end - 1, 0, &end);
- if (!VEC_empty (memory_write_request_s, result)
- && VEC_last (memory_write_request_s, result)->end >= begin)
- {
- VEC_last (memory_write_request_s, result)->end = end;
- }
+ if (!result.empty () && result.back ().end >= begin)
+ result.back ().end = end;
else
{
- struct memory_write_request *n =
- VEC_safe_push (memory_write_request_s, result, NULL);
+ memory_write_request n;
- memset (n, 0, sizeof (struct memory_write_request));
- n->begin = begin;
- n->end = end;
+ memset (&n, 0, sizeof (struct memory_write_request));
+ n.begin = begin;
+ n.end = end;
+
+ result.push_back (n);
}
}
@@ -196,14 +180,14 @@ blocks_to_erase (VEC(memory_write_request_s) *written)
that will be erased but not rewritten (e.g. padding within a block
which is only partially filled by "load"). */
-static VEC(memory_write_request_s) *
-compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks,
- VEC(memory_write_request_s) *written_blocks)
+static std::vector<memory_write_request>
+compute_garbled_blocks (const std::vector<memory_write_request> &erased_blocks,
+ const std::vector<memory_write_request> &written_blocks)
{
- VEC(memory_write_request_s) *result = NULL;
+ std::vector<memory_write_request> result;
- unsigned i, j;
- unsigned je = VEC_length (memory_write_request_s, written_blocks);
+ unsigned j;
+ unsigned je = written_blocks.size ();
struct memory_write_request *erased_p;
/* Look at each erased memory_write_request in turn, and
@@ -213,19 +197,15 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks,
the lists are sorted at this point it could be rewritten more
efficiently, but the complexity is not generally worthwhile. */
- for (i = 0;
- VEC_iterate (memory_write_request_s, erased_blocks, i, erased_p);
- ++i)
+ for (const memory_write_request &erased_iter : erased_blocks)
{
/* Make a deep copy -- it will be modified inside the loop, but
we don't want to modify original vector. */
- struct memory_write_request erased = *erased_p;
+ struct memory_write_request erased = erased_iter;
for (j = 0; j != je;)
{
- struct memory_write_request *written
- = VEC_index (memory_write_request_s,
- written_blocks, j);
+ const memory_write_request *written = &written_blocks[j];
/* Now try various cases. */
@@ -242,7 +222,7 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks,
blocks. */
if (written->begin >= erased.end)
{
- VEC_safe_push (memory_write_request_s, result, &erased);
+ result.push_back (erased);
goto next_erased;
}
@@ -259,12 +239,12 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks,
again for the remainder. */
if (written->begin > erased.begin)
{
- struct memory_write_request *n =
- VEC_safe_push (memory_write_request_s, result, NULL);
+ struct memory_write_request n;
- memset (n, 0, sizeof (struct memory_write_request));
- n->begin = erased.begin;
- n->end = written->begin;
+ memset (&n, 0, sizeof (struct memory_write_request));
+ n.begin = erased.begin;
+ n.end = written->begin;
+ result.push_back (n);
erased.begin = written->begin;
continue;
}
@@ -283,7 +263,7 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks,
/* If we ran out of write requests without doing anything about
ERASED, then that means it's really erased. */
- VEC_safe_push (memory_write_request_s, result, &erased);
+ result.push_back (erased);
next_erased:
;
@@ -292,59 +272,30 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks,
return result;
}
-static void
-cleanup_request_data (void *p)
-{
- VEC(memory_write_request_s) **v = (VEC(memory_write_request_s) **) p;
- struct memory_write_request *r;
- int i;
-
- for (i = 0; VEC_iterate (memory_write_request_s, *v, i, r); ++i)
- xfree (r->data);
-}
-
-static void
-cleanup_write_requests_vector (void *p)
-{
- VEC(memory_write_request_s) **v = (VEC(memory_write_request_s) **) p;
-
- VEC_free (memory_write_request_s, *v);
-}
-
int
-target_write_memory_blocks (VEC(memory_write_request_s) *requests,
+target_write_memory_blocks (const std::vector<memory_write_request> &requests,
enum flash_preserve_mode preserve_flash_p,
void (*progress_cb) (ULONGEST, void *))
{
- struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
- VEC(memory_write_request_s) *blocks = VEC_copy (memory_write_request_s,
- requests);
- unsigned i;
- int err = 0;
+ std::vector<memory_write_request> blocks = requests;
struct memory_write_request *r;
- VEC(memory_write_request_s) *regular = NULL;
- VEC(memory_write_request_s) *flash = NULL;
- VEC(memory_write_request_s) *erased, *garbled;
+ std::vector<memory_write_request> regular;
+ std::vector<memory_write_request> flash;
+ std::vector<memory_write_request> erased, garbled;
/* END == 0 would represent wraparound: a write to the very last
byte of the address space. This file was not written with that
possibility in mind. This is fixable, but a lot of work for a
rare problem; so for now, fail noisily here instead of obscurely
later. */
- for (i = 0; VEC_iterate (memory_write_request_s, requests, i, r); ++i)
- gdb_assert (r->end != 0);
-
- make_cleanup (cleanup_write_requests_vector, &blocks);
+ for (const memory_write_request &iter : requests)
+ gdb_assert (iter.end != 0);
/* Sort the blocks by their start address. */
- qsort (VEC_address (memory_write_request_s, blocks),
- VEC_length (memory_write_request_s, blocks),
- sizeof (struct memory_write_request), compare_block_starting_address);
+ std::sort (blocks.begin (), blocks.end (), compare_block_starting_address);
/* Split blocks into list of regular memory blocks,
and list of flash memory blocks. */
- make_cleanup (cleanup_write_requests_vector, ®ular);
- make_cleanup (cleanup_write_requests_vector, &flash);
split_regular_and_flash_blocks (blocks, ®ular, &flash);
/* If a variable is added to forbid flash write, even during "load",
@@ -354,37 +305,35 @@ target_write_memory_blocks (VEC(memory_write_request_s) *requests,
/* Find flash blocks to erase. */
erased = blocks_to_erase (flash);
- make_cleanup (cleanup_write_requests_vector, &erased);
/* Find what flash regions will be erased, and not overwritten; then
either preserve or discard the old contents. */
garbled = compute_garbled_blocks (erased, flash);
- make_cleanup (cleanup_request_data, &garbled);
- make_cleanup (cleanup_write_requests_vector, &garbled);
- if (!VEC_empty (memory_write_request_s, garbled))
+ std::vector<gdb::unique_xmalloc_ptr<gdb_byte>> mem_holders;
+ if (!garbled.empty ())
{
if (preserve_flash_p == flash_preserve)
{
- struct memory_write_request *r;
-
/* Read in regions that must be preserved and add them to
the list of blocks we read. */
- for (i = 0; VEC_iterate (memory_write_request_s, garbled, i, r); ++i)
+ for (memory_write_request &iter : garbled)
{
- gdb_assert (r->data == NULL);
- r->data = (gdb_byte *) xmalloc (r->end - r->begin);
- err = target_read_memory (r->begin, r->data, r->end - r->begin);
+ gdb_assert (iter.data == NULL);
+ gdb::unique_xmalloc_ptr<gdb_byte> holder
+ ((gdb_byte *) xmalloc (iter.end - iter.begin));
+ iter.data = holder.get ();
+ mem_holders.push_back (std::move (holder));
+ int err = target_read_memory (iter.begin, iter.data,
+ iter.end - iter.begin);
if (err != 0)
- goto out;
+ return err;
- VEC_safe_push (memory_write_request_s, flash, r);
+ flash.push_back (iter);
}
- qsort (VEC_address (memory_write_request_s, flash),
- VEC_length (memory_write_request_s, flash),
- sizeof (struct memory_write_request),
- compare_block_starting_address);
+ std::sort (flash.begin (), flash.end (),
+ compare_block_starting_address);
}
}
@@ -398,47 +347,44 @@ target_write_memory_blocks (VEC(memory_write_request_s) *requests,
have the opportunity to batch flash requests. */
/* Write regular blocks. */
- for (i = 0; VEC_iterate (memory_write_request_s, regular, i, r); ++i)
+ for (const memory_write_request &iter : regular)
{
LONGEST len;
len = target_write_with_progress (current_target.beneath,
TARGET_OBJECT_MEMORY, NULL,
- r->data, r->begin, r->end - r->begin,
- progress_cb, r->baton);
- if (len < (LONGEST) (r->end - r->begin))
+ iter.data, iter.begin,
+ iter.end - iter.begin,
+ progress_cb, iter.baton);
+ if (len < (LONGEST) (iter.end - iter.begin))
{
/* Call error? */
- err = -1;
- goto out;
+ return -1;
}
}
- if (!VEC_empty (memory_write_request_s, erased))
+ if (!erased.empty ())
{
/* Erase all pages. */
- for (i = 0; VEC_iterate (memory_write_request_s, erased, i, r); ++i)
- target_flash_erase (r->begin, r->end - r->begin);
+ for (const memory_write_request &iter : erased)
+ target_flash_erase (iter.begin, iter.end - iter.begin);
/* Write flash data. */
- for (i = 0; VEC_iterate (memory_write_request_s, flash, i, r); ++i)
+ for (const memory_write_request &iter : flash)
{
LONGEST len;
len = target_write_with_progress (¤t_target,
TARGET_OBJECT_FLASH, NULL,
- r->data, r->begin,
- r->end - r->begin,
- progress_cb, r->baton);
- if (len < (LONGEST) (r->end - r->begin))
+ iter.data, iter.begin,
+ iter.end - iter.begin,
+ progress_cb, iter.baton);
+ if (len < (LONGEST) (iter.end - iter.begin))
error (_("Error writing data to flash"));
}
target_flash_done ();
}
- out:
- do_cleanups (back_to);
-
- return err;
+ return 0;
}
diff --git a/gdb/target.h b/gdb/target.h
index 83cf48575f..7636685f30 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1473,8 +1473,6 @@ struct memory_write_request
/* A callback baton for progress reporting for this request. */
void *baton;
};
-typedef struct memory_write_request memory_write_request_s;
-DEF_VEC_O(memory_write_request_s);
/* Enumeration specifying different flash preservation behaviour. */
enum flash_preserve_mode
@@ -1500,9 +1498,10 @@ enum flash_preserve_mode
with a NULL baton, when preserved flash sectors are being rewritten.
The function returns 0 on success, and error otherwise. */
-int target_write_memory_blocks (VEC(memory_write_request_s) *requests,
- enum flash_preserve_mode preserve_flash_p,
- void (*progress_cb) (ULONGEST, void *));
+int target_write_memory_blocks
+ (const std::vector<memory_write_request> &requests,
+ enum flash_preserve_mode preserve_flash_p,
+ void (*progress_cb) (ULONGEST, void *));
/* Print a line about the current target. */
--
2.13.6