Re: [PATCH 1/6] Introduce target-section.h

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom Tromey <tom@tromey.com> [2020-10-03 13:37:30 -0600]:

> This introduces a new target-section.h file.  This makes some of the
> later patches in this series a bit cleaner, because new includes of
> target.h won't be required.  Also I think it's better to have small
> header files for each separate data structure.
> 
> gdb/ChangeLog
> 2020-10-03  Tom Tromey  <tom@tromey.com>
> 
> 	* target.h (struct target_section, struct target_section_table):
> 	Move to target-section.h.
> 	* target-section.h: New file.
> ---
>  gdb/ChangeLog        |  6 ++++++
>  gdb/target-section.h | 50 ++++++++++++++++++++++++++++++++++++++++++++
>  gdb/target.h         | 28 +------------------------
>  3 files changed, 57 insertions(+), 27 deletions(-)
>  create mode 100644 gdb/target-section.h
> 
> diff --git a/gdb/target-section.h b/gdb/target-section.h
> new file mode 100644
> index 00000000000..581bc1dbe9f
> --- /dev/null
> +++ b/gdb/target-section.h
> @@ -0,0 +1,50 @@
> +/* Target sections.
> +
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GDB_TARGET_SECTION_H
> +#define GDB_TARGET_SECTION_H
> +
> +/* Struct target_section maps address ranges to file sections.  It is
> +   mostly used with BFD files, but can be used without (e.g. for handling
> +   raw disks, or files not in formats handled by BFD).  */
> +
> +struct target_section
> +  {
> +    CORE_ADDR addr;		/* Lowest address in section */
> +    CORE_ADDR endaddr;		/* 1+highest address in section */
> +
> +    struct bfd_section *the_bfd_section;
> +
> +    /* The "owner" of the section.
> +       It can be any unique value.  It is set by add_target_sections
> +       and used by remove_target_sections.
> +       For example, for executables it is a pointer to exec_bfd and
> +       for shlibs it is the so_list pointer.  */
> +    void *owner;
> +  };

I know that this is copy & paste code, but it would be awesome if we
could take this opportunity to fix the whitespace formatting, place
the comments in the correct place, reword them as proper sentences,
and add a comment to the missing field.

Generally bring this code up to our current coding standard :)

Otherwise this looks great.

Thanks,
Andrew


> +
> +/* Holds an array of target sections.  Defined by [SECTIONS..SECTIONS_END[.  */
> +
> +struct target_section_table
> +{
> +  struct target_section *sections;
> +  struct target_section *sections_end;
> +};
> +
> +#endif /* GDB_TARGET_SECTION_H */
> diff --git a/gdb/target.h b/gdb/target.h
> index 695f1b2b7f6..369394aa929 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -44,6 +44,7 @@ struct inferior;
>  #include "breakpoint.h" /* For enum bptype.  */
>  #include "gdbsupport/scoped_restore.h"
>  #include "gdbsupport/refcounted-object.h"
> +#include "target-section.h"
>  
>  /* This include file defines the interface between the main part
>     of the debugger, and the part which is target-specific, or
> @@ -2413,33 +2414,6 @@ extern bool target_is_pushed (target_ops *t);
>  extern CORE_ADDR target_translate_tls_address (struct objfile *objfile,
>  					       CORE_ADDR offset);
>  
> -/* Struct target_section maps address ranges to file sections.  It is
> -   mostly used with BFD files, but can be used without (e.g. for handling
> -   raw disks, or files not in formats handled by BFD).  */
> -
> -struct target_section
> -  {
> -    CORE_ADDR addr;		/* Lowest address in section */
> -    CORE_ADDR endaddr;		/* 1+highest address in section */
> -
> -    struct bfd_section *the_bfd_section;
> -
> -    /* The "owner" of the section.
> -       It can be any unique value.  It is set by add_target_sections
> -       and used by remove_target_sections.
> -       For example, for executables it is a pointer to exec_bfd and
> -       for shlibs it is the so_list pointer.  */
> -    void *owner;
> -  };
> -
> -/* Holds an array of target sections.  Defined by [SECTIONS..SECTIONS_END[.  */
> -
> -struct target_section_table
> -{
> -  struct target_section *sections;
> -  struct target_section *sections_end;
> -};
> -
>  /* Return the "section" containing the specified address.  */
>  struct target_section *target_section_by_addr (struct target_ops *target,
>  					       CORE_ADDR addr);
> -- 
> 2.17.2
>