On Friday 21 November 2008 06:25:10 Cal Page wrote:
> These come from yaffs_ScanBackwards where chunk is calculated as
>
>   chunk = blk * dev->nChunksPerBlock + c
>
> but blk is [1..n] not zero based, so shouldn't it be
>
>   chunk = (blk-1) * ...
>
> (IMHO, egad, you've got some 0 based indexes and some 1 based indexes.
> It's a sure fire way to introduce bugs. For cleanup, you SHOULD move
> everything to 0 based indexes.)

No you have that wrong because blk does not necessarily start at 1 but at an 
arbitrary value defined by dev->internalStartBlock

To understand why the code is this way, you probably also need to understand 
some historic context. Long, long ago, block numbers were always 1 or greater 
with zero being reserved as the value for undefined/illegal value (ie. 
dev->startBlock had to be 1 or more). This was one consistent definition 
of "block number" right through yaffs. This however meant that yaffs did not 
use block zero of the flash device, which upset people using yaffs in small 
systems with only very few blocks. 

Rather than go through the whole of yaffs changing these numbers, I untroduced 
the idea of an internal block number (that most of yaffs uses) and an 
external one (that is aligned to the hardware) with an offset. If someone 
uses dev->startBlock of zero then an offset of 1 block is used. This is 
reasonably clean if you understand where it is applied, but is likely a bit 
confusing if you don't understand the rationale.

That code is working fine in many shipping products. While I won't say that 
yaffs is bug free, I would suggest that if you are finding an urge to change 
a lot of things like this then likely something else is broken.