On Tuesday 01 August 2006 02:08, Mark Kettenis wrote:


> > +          if (!VEC_empty (memory_region, result))
> > +            for (i = 0; i < VEC_length (memory_region, result) - 1; ++i)
> > +              {
> > +                memory_region *this_one = VEC_index (memory_region,
> > result, i); +                memory_region *next_one = VEC_index
> > +                  (memory_region, result, i+1);
>
> Missing spaces around the '+'.

Ok.


> > === gdb/target.h
> > ==================================================================
> > --- gdb/target.h	(/mirrors/gdb)	(revision 326)
> > +++ gdb/target.h	(/patches/memory_map/gdb)	(revision 326)
> > @@ -55,6 +55,8 @@
> >  #include "symtab.h"
> >  #include "dcache.h"
> >  #include "memattr.h"
> > +#include "vec.h"
> > +#include "memory-map.h"
> >
> >  enum strata
> >    {
> > @@ -194,7 +196,9 @@
> >    /* Transfer auxilliary vector.  */
> >    TARGET_OBJECT_AUXV,
> >    /* StackGhost cookie.  See "sparc-tdep.c".  */
> > -  TARGET_OBJECT_WCOOKIE
> > +  TARGET_OBJECT_WCOOKIE,
> > +  /* Target memory map in XML format.  */
> > +  TARGET_OBJECT_MEMORY_MAP,
>
> I'm still not sure how this fits in.  Certainly if my target already
> provides a memory map in a nice data structure I'm not supposed to
> convert that into XML am I?  I should be able to just implement
> to_memory_map and convert it directly into a VEC(memory_region).

Yes. The remote implementation of to_memory_map uses TARGET_OBJECT_MEMORY_MAP,
but implementation for another target is not required to.

> Arghh, I just realize that memory_region is a typedef for struct
> memory_region.  I'm not too big a fan of that practice, since I stop
> realizing that it really is a struct and start doing stupid things
> with it...

Well, "struct memory_region" is too long and really a lot of lines will run 
out of 80 columns immediately.

> >    /* Possible future objects: TARGET_OBJECT_FILE, TARGET_OBJECT_PROC,
> > ... */ };
> > @@ -437,6 +441,20 @@
> >  				gdb_byte *readbuf, const gdb_byte *writebuf,
> >  				ULONGEST offset, LONGEST len);
> >
> > +    /* Returns the memory map for the target. The return value of 0
> > means +       that no memory map is available. If a memory address does
> > not fall +       within any returned regions, it's assumed to be RAM. 
> > The returned +       memory regions should not overlap.
> > +
> > +       The order of regions does not matter, as target_memory_map will
> > +       sort regions by starting address anyway. For that reason, this
> > +       function should not be called directly, only via
> > target_memory_map. +
> > +       This method is expected to cache the data if fetching it is slow.
> > +       The higher-level code has no way of knowing when memory map
> > +       could change, and so can't do caching itself.  */
> > +    VEC(memory_region) * (*to_memory_map) (struct target_ops *);
>
> That's not a multiplication isn't it?  I think you should remove the
> space after that first '*' (indent is too stupid and parses this as a
> binary operator).

Ok.

>
> > +
> > +/* Returns the value of attribute ATTR from expat attribute list ATTRS.
> > +   If not found, calls 'error'.  */
> > +const XML_Char *xml_get_attribute_value(const XML_Char **attrs,
> > +                                        const XML_Char *attr)
> > +{
>
> const XML_Char *
> xml_get_attr...
>
> > +
> > +int compare_memory_region_starting_address (const void* a, const void
> > *b) +{
> > +  ULONGEST a_begin = ((memory_region *)a)->begin;
> > +  ULONGEST b_begin = ((memory_region *)b)->begin;
> > +  return a_begin - b_begin;
> > +}
>
> int
> compare_memort_region...

OK.

> > === gdb/memory-map.h
> > ==================================================================
> > --- gdb/memory-map.h	(/mirrors/gdb)	(revision 326)
> > +++ gdb/memory-map.h	(/patches/memory_map/gdb)	(revision 326)
> > @@ -0,0 +1,142 @@
> > +/* Routines for handling XML memory maps provided by target.
> > +
> > +   Copyright (C) 2006
> > +   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 2 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, write to the Free Software
> > +   Foundation, Inc., 51 Franklin Street, Fifth Floor,
> > +   Boston, MA 02110-1301, USA.  */
> > +
> > +
> > +#ifndef MEMORY_MAP_H
> > +#define MEMORY_MAP_H
> > +
> > +#include "vec.h"
> > +
> > +/* Describes various kinds of memory regions.  */
> > +enum memory_region_type
> > +  {
> > +    /* Memory that can be freely written and read.  */
> > +    TARGET_MEMORY_RAM,
> > +    /* Memory that can't be written at all.  */
> > +    TARGET_MEMORY_ROM,
> > +    /* Memory that can be written only using special operations.  */
> > +    TARGET_MEMORY_FLASH
> > +  };
> > +
> > +/* Describes properties of a memory range.  */
> > +typedef struct memory_region
> > +  {
> > +    /* The first address of the region.  */
> > +    ULONGEST begin;
> > +    /* The past-the-end address of the region.  */
> > +    ULONGEST end;
> > +    /* Type of the memory in this region.  */
> > +    enum memory_region_type memory_type;
> > +    /* The size of flash memory sector.  Some flash chips have
> > non-uniform +       sector sizes, for example small sectors at beginning
> > and end. +       In this case gdb will have to have several memory_region
> > objects each +       one having uniform sector size.
> > +       This field is defined only of MEMORY_TYPE == TARGET_MEMORY_FLASH.
> >  */ +    unsigned flash_block_size;
> > +  } memory_region;
> > +
> > +DEF_VEC_O(memory_region);
> > +
> > +/* Casts both A and B to memory_region, compares they starting addresses
> > +   and returns value less than zero, equal to zero, or greater then zero
> > +   if A's starting address is less than B's starting address, equal to,
> > +   or greater then, respectively.  This function is suitable for sorting
> > +   vector of memory_regions with the qsort function.  */
> > +int compare_memory_region_starting_address (const void* a, const void
> > *b); +
> > +/* Parses XML memory map passed as argument and returns the memory
> > +   regions it describes.  On any error, emits error message and
> > +   returns 0. Does not throw.  Ownership of result is passed to the
> > caller.  */ +VEC(memory_region) *parse_memory_map (const char
> > *memory_map);
> > +
> > +#endif
> > +/* Routines for handling XML memory maps provided by target.
.....
> You really don't have to say things twice ;-).

Ick. Either "patch" or "svk" is playing tricks on me.

Revised patch attached (changelog unchaged).

- Volodya