From patchwork Sun Feb 25 22:26:03 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 26052 Received: (qmail 127495 invoked by alias); 25 Feb 2018 22:26:07 -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 127478 invoked by uid 89); 25 Feb 2018 22:26:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=time_point X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 25 Feb 2018 22:26:05 +0000 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id CD54A1E584; Sun, 25 Feb 2018 17:26:03 -0500 (EST) Subject: Re: [RFA] Change target_write_memory_blocks to use std::vector To: Tom Tromey , gdb-patches@sourceware.org References: <20180225173703.6675-1-tom@tromey.com> From: Simon Marchi Message-ID: <05afbce6-b052-9cb4-d4bc-c392f992ab60@simark.ca> Date: Sun, 25 Feb 2018 17:26:03 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180225173703.6675-1-tom@tromey.com> On 2018-02-25 12:37 PM, Tom Tromey wrote: > - 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 requests; > + > + load_section_data () > + { > + } Is this empty constructor needed? Actually, I think it would be nice to give constructors to the data structures when possible, to make it less likely to have them in invalid states. Here's an example, you can integrate it in your patch if you like it. Simon From 59f9a9d763dc4c0a879f1d36696efa2c4d423c86 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 25 Feb 2018 17:10:18 -0500 Subject: [PATCH] Add constructors to structures --- gdb/symfile.c | 99 +++++++++++++++++++++++++---------------------------- gdb/target-memory.c | 17 ++------- gdb/target.h | 25 ++++++++------ 3 files changed, 63 insertions(+), 78 deletions(-) diff --git a/gdb/symfile.c b/gdb/symfile.c index 104d149584..cb1e44d605 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -1892,46 +1892,56 @@ add_section_size_callback (bfd *abfd, asection *asec, void *data) *sum += bfd_get_section_size (asec); } -/* Opaque data for load_section_callback. */ -struct load_section_data { - CORE_ADDR load_offset = 0; - struct load_progress_data *progress_data = nullptr; - std::vector requests; - - load_section_data () - { - } - - ~load_section_data () - { - for (auto &&request : requests) - { - xfree (request.data); - xfree (request.baton); - } - } -}; - /* Opaque data for load_progress. */ -struct load_progress_data { +struct load_progress_data +{ /* Cumulative data. */ - unsigned long write_count; - unsigned long data_count; - bfd_size_type total_size; + unsigned long write_count = 0; + unsigned long data_count = 0; + bfd_size_type total_size = 0; }; /* Opaque data for load_progress for a single section. */ -struct load_progress_section_data { +struct load_progress_section_data +{ + load_progress_section_data (load_progress_data *cumulative_, + const char *section_name_, ULONGEST section_size_, + CORE_ADDR lma_, gdb_byte *buffer_) + : cumulative (cumulative_), section_name (section_name_), + section_size (section_size_), lma (lma_), buffer (buffer_) + {} + struct load_progress_data *cumulative; /* Per-section data. */ const char *section_name; - ULONGEST section_sent; + ULONGEST section_sent = 0; ULONGEST section_size; CORE_ADDR lma; gdb_byte *buffer; }; +/* Opaque data for load_section_callback. */ +struct load_section_data +{ + load_section_data (load_progress_data *progress_data_) + : progress_data (progress_data_) + {} + + ~load_section_data () + { + for (auto &&request : requests) + { + xfree (request.data); + delete ((load_progress_section_data *) request.baton); + } + } + + CORE_ADDR load_offset = 0; + struct load_progress_data *progress_data; + std::vector requests; +}; + /* Target write callback routine for progress reporting. */ static void @@ -2001,11 +2011,8 @@ 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 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); - gdb_byte *buffer; const char *sect_name = bfd_get_section_name (abfd, asec); if ((bfd_get_section_flags (abfd, asec) & SEC_LOAD) == 0) @@ -2014,25 +2021,16 @@ load_section_callback (bfd *abfd, asection *asec, void *data) if (size == 0) return; - 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; - - 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->buffer = buffer; + ULONGEST begin = bfd_section_lma (abfd, asec) + args->load_offset; + ULONGEST end = begin + size; + gdb_byte *buffer = (gdb_byte *) xmalloc (size); + bfd_get_section_contents (abfd, asec, buffer, 0, size); - args->requests.push_back (new_request); + load_progress_section_data *section_data + = new load_progress_section_data (args->progress_data, sect_name, size, + begin, buffer); - bfd_get_section_contents (abfd, asec, buffer, 0, size); + args->requests.emplace_back (begin, end, buffer, section_data); } static void print_transfer_performance (struct ui_file *stream, @@ -2043,15 +2041,10 @@ static void print_transfer_performance (struct ui_file *stream, void generic_load (const char *args, int from_tty) { - struct load_section_data cbdata; struct load_progress_data total_progress; + struct load_section_data cbdata (&total_progress); struct ui_out *uiout = current_uiout; - CORE_ADDR entry; - - memset (&total_progress, 0, sizeof (total_progress)); - cbdata.progress_data = &total_progress; - if (args == NULL) error_no_arg (_("file to load")); @@ -2100,7 +2093,7 @@ generic_load (const char *args, int from_tty) steady_clock::time_point end_time = steady_clock::now (); - entry = bfd_get_start_address (loadfile_bfd.get ()); + CORE_ADDR entry = bfd_get_start_address (loadfile_bfd.get ()); entry = gdbarch_addr_bits_remove (target_gdbarch (), entry); uiout->text ("Start address "); uiout->field_fmt ("address", "%s", paddress (target_gdbarch (), entry)); diff --git a/gdb/target-memory.c b/gdb/target-memory.c index a945983723..395bf0bae9 100644 --- a/gdb/target-memory.c +++ b/gdb/target-memory.c @@ -160,15 +160,7 @@ blocks_to_erase (const std::vector &written) if (!result.empty () && result.back ().end >= begin) result.back ().end = end; else - { - memory_write_request n; - - memset (&n, 0, sizeof (struct memory_write_request)); - n.begin = begin; - n.end = end; - - result.push_back (n); - } + result.emplace_back (begin, end); } return result; @@ -239,12 +231,7 @@ compute_garbled_blocks (const std::vector &erased_blocks, again for the remainder. */ if (written->begin > erased.begin) { - struct memory_write_request n; - - memset (&n, 0, sizeof (struct memory_write_request)); - n.begin = erased.begin; - n.end = written->begin; - result.push_back (n); + result.emplace_back (erased.begin, written->begin); erased.begin = written->begin; continue; } diff --git a/gdb/target.h b/gdb/target.h index 7636685f30..452855ae50 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1463,16 +1463,21 @@ void target_flash_done (void); /* Describes a request for a memory write operation. */ struct memory_write_request - { - /* Begining address that must be written. */ - ULONGEST begin; - /* Past-the-end address. */ - ULONGEST end; - /* The data to write. */ - gdb_byte *data; - /* A callback baton for progress reporting for this request. */ - void *baton; - }; +{ + memory_write_request (ULONGEST begin_, ULONGEST end_, + gdb_byte *data_ = nullptr, void *baton_ = nullptr) + : begin (begin_), end (end_), data (data_), baton (baton_) + {} + + /* Begining address that must be written. */ + ULONGEST begin; + /* Past-the-end address. */ + ULONGEST end; + /* The data to write. */ + gdb_byte *data; + /* A callback baton for progress reporting for this request. */ + void *baton; +}; /* Enumeration specifying different flash preservation behaviour. */ enum flash_preserve_mode