On 7/4/07, eran liberty eran.liberty@gmail.com wrote:
On 7/3/07, Grant Likely grant.likely@secretlab.ca wrote:
On 7/3/07, eran.liberty@gmail.com eran.liberty@gmail.com wrote:
--- 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.
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.
My 'nitpick' is about the for loop becoming longer than 80chars, and hence being wrapped to 3 lines which makes it harder to read. I'd prefer code that keeps the for(;;) statement short for readability.
g.