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 <[email protected]>
> > ---
> >  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
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to