[U-Boot-Users] [PATCH] cfi_flash.c bug fix

[eran liberty]

On 7/3/07, Grant Likely <grant.likely at secretlab.ca> wrote:
> On 7/3/07, eran.liberty at gmail.com <eran.liberty at gmail.com> wrote:
> > this fix a possible bug which occur when the config file defines a flash which is actually smaller than the physical one.
> > on top of that, if you want to force your flash to appear smaller you can define CONFIG_FORCE_FLASH_BANK_SIZE the a corresponding value.
> >
> > Signed-off-by: Eran Liberty <eran.liberty at gmail.com>
> >
> > Index: drivers/cfi_flash.c
> > ===================================================================
> > --- drivers/cfi_flash.c (.../tags/trunk/20070620_2_merge_to_exsw6000)   (revision 69)
> > +++ drivers/cfi_flash.c (.../branches/exsw6000) (revision 69)
> > @@ -1281,7 +1281,9 @@
> >                         erase_region_count = (tmp & 0xffff) + 1;
> >                         debug ("erase_region_count = %d erase_region_size = %d\n",
> >                                 erase_region_count, erase_region_size);
> > -                       for (j = 0; j < erase_region_count; j++) {
> > +                       for (j = 0;
> > +                            j < erase_region_count
> > +                            && sect_cnt < CFG_MAX_FLASH_SECT; j++) {
> >                                 info->start[sect_cnt] = sector;
> >                                 sector += (erase_region_size * size_ratio);
> >
> Nit: rather than adding a test in the for loop, can you modify
> erase_region_count before the loop to reflect the maximum size?  I
> think it will result in cleaner code.
>
> ie:
> if (erase_region_count > CFG_MAX_FLASH_SECT - sect_cnt)
>         erase_region_count = CFG_MAX_FLASH_SECT - sect_cnt;
> (but double check that I haven't created an off-by-one error)
>
> Cheers,
> g.
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
> grant.likely at secretlab.ca
> (403) 399-0195
>

the code with my patch flows like this...
sect_cnt = 0;
for (i = 0; i < num_erase_regions; i++) {
       ...
       ...
       erase_region_count = (tmp & 0xffff) + 1;
       for (j = 0; j < erase_region_count && sect_cnt <
CFG_MAX_FLASH_SECT; j++){
	      ...
	      ...
	      sect_cnt++;
      }
}

If I had not missed anything,
your suggestion tie together two variables which are not necessarily
connected. True, your suggestion will prevent the j variable from
letting the sect_cnt overflow in the , should be last, iteration.
But i feel it is circumventive and less straight forward.
The variable causing the spill over is sect_cnt, there for it should
be the one that being tested against its own boundaries.

"Happy Happy Joy Joy",
Liberty