Re: [PATCH] Re: Flash memory size not aligned to address

[Kevin Buettner <kevinb at redhat dot com>]

On Mon, 9 Oct 2017 10:28:13 -0600
Mark Rages <markrages@gmail.com> wrote:

> > Do you have an FSF Copyright Assignment?
> >  
> 
> No.

According to this link...

https://www.gnu.org/prep/maintain/maintain.html#Legally-Significant

...contributions that consist of more than about 15 lines of code or
text are significant for copyright purposes.

Your patch is well under that amount, so I do not think that you
*need* a copyright assignment for this particular change.

However, if you plan to make other contributions to GDB, I encourage
you to have a copyright assignment on file with the FSF.

Please let us know if you intend to pursue a copyright assignment.
If not, I can commit/push this change for you.

> > ChangeLog entries are generally not posted as patches.  It is unlikely
> > that this portion of the patch will cleanly apply once it's approved.  
> 
> Ok.  Thank you for pointing me to the wiki.  I was going off of
> gdb/CONTRIBUTE.  Which one is authoritative?

I think that gdb/CONTRIBUTE is meant to be authoritative.

But I find that the wiki document provides some really useful nuts
and bolts type of stuff for contributing to GDB.  Personally, I find
it to be a lot more useful than gdb/CONTRIBUTE.

For example, the wiki contains some really useful info on how to
format your commit comment when using git.  It also contains some
good information on formatting ChangeLog entries.

> > It occurs to me that address_in_region might be better named
> > offset_in_region.  
> 
> Updated patch follows.
> 
> > It seems to me that there will be no difference in behavior for
> > targets whose region boundaries are already aligned to the block size.
> > I do wonder, however, about behavior on other targets that don't meet
> > this criteria.  I'm hoping that someone else who has more experience in
> > this area will comment.  
> 
> The only situation I can think of would be a device that begins a
> section of flash unaligned, but requires an aligned address for the
> erase command.  I'm not aware of such, but it's a big world.
> 
> Anyway, thanks for looking it over. Here goes second try:
> 
> gdb/ChangeLog:
> 
>         * target-memory.c (block_boundaries): Fix for block address not
>         aligned on block size.
> 
> diff --git a/gdb/target-memory.c b/gdb/target-memory.c
> index 1c8faa8..7f048de 100644
> --- a/gdb/target-memory.c
> +++ b/gdb/target-memory.c
> @@ -138,14 +138,18 @@ block_boundaries (CORE_ADDR address, CORE_ADDR
> *begin, CORE_ADDR *end)
>  {
>    struct mem_region *region;
>    unsigned blocksize;
> +  CORE_ADDR offset_in_region;
> 
>    region = lookup_mem_region (address);
>    gdb_assert (region->attrib.mode == MEM_FLASH);
>    blocksize = region->attrib.blocksize;
> +
> +  offset_in_region = address - region->lo;
> +
>    if (begin)
> -    *begin = address / blocksize * blocksize;
> +    *begin = region->lo + offset_in_region / blocksize * blocksize;
>    if (end)
> -    *end = (address + blocksize - 1) / blocksize * blocksize;
> +    *end = region->lo + (offset_in_region + blocksize - 1) /
> blocksize * blocksize;
>  }
> 

This is okay.

As noted earlier, please let us know if you intend to pursue an FSF
copyright assignment.  If so, in order have write access to the git
repository, you'll need a sourceware account.  You should also submit
a patch adding yourself to the "Write After Approval" section of
gdb/MAINTAINERS.  (We can go into how all of that is accomplished
after the copyright assignment process is finished.)

Kevin