On Tuesday 01 August 2006 02:21, Mark Kettenis wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Mon, 31 Jul 2006 17:30:56 +0400
> >
> > === gdb/target.c
> > ==================================================================
> > --- gdb/target.c	(/mirrors/gdb)	(revision 332)
> > +++ gdb/target.c	(/patches/flash_memory/gdb)	(revision 332)
> > @@ -1350,7 +1387,7 @@
> >    return target_xfer_partial (ops, object, annex, buf, NULL, offset,
> > len); }
> >
> > -static LONGEST
> > +LONGEST
> >  target_write_partial (struct target_ops *ops,
> >  		      enum target_object object,
> >  		      const char *annex, const gdb_byte *buf,
>
> Uh, we just made that one static.  Merge botch?

No, this is intentional change. The target_write_memory_blocks uses it, and it 
was made explicitly so that we can produce progress report while loading 
data. It's currently used by MI frontend, and if target_write_memory_blocks 
is coded to call progress reporting routine only at the end of a section, it 
will make progress reporting much less usefull.

> > === gdb/symfile.c
> > ==================================================================
> > --- gdb/symfile.c	(/mirrors/gdb)	(revision 332)
> > +++ gdb/symfile.c	(/patches/flash_memory/gdb)	(revision 332)
> > @@ -1512,15 +1512,6 @@
> >    overlay_cache_invalid = 1;
> >  }
......
> > +  if (target_write_memory_blocks
> > +      (cbdata.memory_write_requests,
> > +       &allow_flash_write, &dont_preserve_flash, &download_progress_cb)
> > != 0) +    {
> > +      error (_("Failed to write memory"));
> > +    }
> > +
>
> This really confuses me.  It looks like this completely rewrites the
> "load" functionality, putting bfd completely out of the picture.  Why?
> How does this change a load into non-flash memory?

It does not put removes use of bfd in fact. Diff is not very helpfull, so I 
attach the new version of the file, take a look at "generic_load" function.

The existing 'generic_load' first gets section addresses and data from bfd, 
and them immediately writes them. That would not work with flash, because 
then stub won't have any idea if you've any more data follow. If you have 
2 writes going into the same flash sector, then either second write will erase 
whatever the first one has written, or stub has to use complicated or 
inefficient approaches. So, we need to collect all write request, and then 
process them together.

So, generic_load first creates VEC(memory_write_request) and then passes it on
to target_write_memory_blocks. If no memory_write_request if found to touch 
flash memory, target_write_memory_block just writes them directly, see code  
after 
 
   /* Write regular blocks.  */

comment. 


> > +/* Given a list of blocks that will be erased with flash erase commands,
> > +   and the list of blocks that will be written, computed the set
> > +   of regions that will have its content erased and not written.  */
> > +static VEC(memory_write_request) *
> > +compute_garbled_blocks(VEC(memory_write_request)* erased_blocks,
> > +                       VEC(memory_write_request)* written_blocks)
>
> Missing sace before the first '('.

Ok.

> > +{
> > +  VEC(memory_write_request) *result = NULL;
> > +
> > +  unsigned i, j;
> > +  unsigned je = VEC_length (memory_write_request, written_blocks);
> > +  memory_write_request *erased_p;
> > +
> > +  /* Look at each erased memory_write_request in turn, and see if what
> > part of it is +     subsequently written to. */
>
> That line is defenitely too long.

Ok.

> > +
> > + out:
> > +  do_cleanups (back_to);
> > +
> > +  return err;
> > +}
> > +
> > +
>
> Please don't add extra whitespace at the end of a file.

Ok.

Updated patch attached.

- Volodya