Re: [Openocd-development] [PATCH] CFI CORE: bug-fix protect single sector

[David B. <da...@pa...>]

On Tuesday 09 March 2010, David Brownell wrote:

And tl clarify a bit:


> On Monday 08 March 2010, Antonio Borneo wrote:
> > Cannot protect or unprotect single sector in cfi flash.
> > When first==last the procedure fails.
> > 
> > Signed-off-by: Antonio Borneo <bor...@gm...>
> > ---
> >  src/flash/nor/core.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/flash/nor/core.c b/src/flash/nor/core.c
> > index 767006d..8b581b0 100644
> > --- a/src/flash/nor/core.c
> > +++ b/src/flash/nor/core.c
> > @@ -73,7 +73,7 @@ int flash_driver_protect(struct flash_bank *bank, int set, int first, int last)
> >  	 * speeds at least some things up.
> >  	 */
> >  scan:
> > -	for (int i = first; i < last; i++) {
> > +	for (int i = first; i <= last; i++) {
> 
> OK 

... because "first == last" is an OK loop entry state;
it was explicitly allowed in the procedure parameter
checks earlier, indicating that "single sector" case.


> 
> >  		struct flash_sector *sector = bank->sectors + i;
> >  
> >  		/* Only filter requests to protect the already-protected, or
> > @@ -108,7 +108,7 @@ scan:
> >  	}
> >  
> >  	/* Single sector, already protected?  Nothing to do! */
> > -	if (first == last)
> > +	if (first > last)
> 
> ... not.  the loop previously maintained the "first <= last" invariant,
> and should still have done so.  "first > last" is clearly an error case.

... "clearly" since the procedure parameter checks rejected that error.
Also,

 - a comment in the loop pointed out the importance of not overrunning
   the end

 - the comment up top says "first == last" is the "single sector" case

In short, this would add three internal inconsistencies...

> 
> 
> >  		return ERROR_OK;
> >  
> >  
> > -- 
> > 1.5.2.2