@@ -97,34 +97,105 @@ void flash_cache_flush (flash_cache_t* fc)
9797
9898void flash_cache_read (flash_cache_t * fc , void * dst , uint32_t addr , uint32_t count )
9999{
100+ // there is no check for overflow / wraparound for dst + count, addr + count.
101+ // this might be a useful thing to add for at least debug builds.
102+
100103 // overwrite with cache value if available
101- if ( (fc -> cache_addr != FLASH_CACHE_INVALID_ADDR ) &&
102- !(addr < fc -> cache_addr && addr + count <= fc -> cache_addr ) &&
103- !(addr >= fc -> cache_addr + FLASH_CACHE_SIZE ) )
104+ if ( (fc -> cache_addr != FLASH_CACHE_INVALID_ADDR ) && // cache is not valid
105+ !(addr < fc -> cache_addr && addr + count <= fc -> cache_addr ) && // starts before, ends before cache area
106+ !(addr >= fc -> cache_addr + FLASH_CACHE_SIZE ) ) // starts after end of cache area
104107 {
105- int dst_off = fc -> cache_addr - addr ;
106- int src_off = 0 ;
107-
108- if ( dst_off < 0 )
108+ // This block is entered only when the read overlaps the cache area by at least one byte.
109+ // If the read starts before the cache area, it's further guaranteed
110+ // that count is large enough to cause the read to enter
111+ // the cache area by at least 1 byte.
112+ uint32_t dst_off = 0 ;
113+ uint32_t src_off = 0 ;
114+ if (addr < fc -> cache_addr )
109115 {
110- src_off = - dst_off ;
111- dst_off = 0 ;
116+ dst_off = fc -> cache_addr - addr ;
117+ // Read the bytes prior to the cache address
118+ fc -> read (dst , addr , dst_off );
112119 }
113-
114- int cache_bytes = minof (FLASH_CACHE_SIZE - src_off , count - dst_off );
115-
116- // start to cached
117- if ( dst_off ) fc -> read (dst , addr , dst_off );
118-
119- // cached
120+ else
121+ {
122+ src_off = addr - fc -> cache_addr ;
123+ }
124+
125+ // Thus, after the above code block executes:
126+ // *** AT MOST ***, only one of src_off and dst_off are non-zero;
127+ // (Both may be zero when the read starts at the start of the cache area.)
128+ // dst_off corresponds to the number of bytes already read from PRIOR to the cache area.
129+ // src_off corresponds to the byte offset to start reading at, from WITHIN the cache area.
130+
131+ // How many bytes to memcpy from flash area?
132+ // Remember that, AT MOST, one of src_off and dst_off are non-zero.
133+ // If src_off is non-zero, then dst_off is zero, representing that the
134+ // read starts inside the cache. In this case:
135+ // PARAM1 := FLASH_CACHE_SIZE - src_off == maximum possible bytes to read from cache
136+ // PARAM2 := count
137+ // Thus, taking the minimum of the two gives the number of bytes to read from cache,
138+ // in the range [ 1 .. FLASH_CACHE_SIZE-src_off ].
139+ // Else if dst_off is non-zero, then src_off is zero, representing that the
140+ // read started prior to the cache area. In this case:
141+ // PARAM1 := FLASH_CACHE_SIZE == full size of the cache
142+ // PARAM2 := count - dst_off == total bytes requested, minus the count of those already read
143+ // Because the original request is guaranteed to overlap the cache, the range for
144+ // PARAM2 is ensured to be [ 1 .. count-1 ].
145+ // Thus, taking the minimum of the two gives the number of bytes to read from cache,
146+ // in the range [ 1 .. FLASH_CACHE_SIZE ]
147+ // Else both src_off and dst_off are zero, representing that the read is starting
148+ // exactly aligned to the cache.
149+ // PARAM1 := FLASH_CACHE_SIZE
150+ // PARAM2 := count
151+ // Thus, taking the minimum of the two gives the number of bytes to read from cache,
152+ // in the range [ 1 .. FLASH_CACHE_SIZE ]
153+ //
154+ // Therefore, in all cases, there is assurance that cache_bytes
155+ // will be in the final range [1..FLASH_CACHE_SIZE].
156+ uint32_t cache_bytes = minof (FLASH_CACHE_SIZE - src_off , count - dst_off );
157+
158+ // Use memcpy to read cached data into the buffer
159+ // If src_off is non-zero, then dst_off is zero, representing that the
160+ // read starts inside the cache. In this case:
161+ // PARAM1 := dst
162+ // PARAM2 := fc->cache_buf + src_off
163+ // PARAM3 := cache_bytes
164+ // Thus, all works as expected when starting in the midst of the cache.
165+ // Else if dst_off is non-zero, then src_off is zero, representing that the
166+ // read started prior to the cache. In this case:
167+ // PARAM1 := dst + dst_off == destination offset by number of bytes already read
168+ // PARAM2 := fc->cache_buf
169+ // PARAM3 := cache_bytes
170+ // Thus, all works as expected when starting prior to the cache.
171+ // Else both src_off and dst_off are zero, representing that the read is starting
172+ // exactly aligned to the cache.
173+ // PARAM1 := dst
174+ // PARAM2 := fc->cache_buf
175+ // PARAM3 := cache_bytes
176+ // Thus, all works as expected when starting exactly at the cache boundary
177+ //
178+ // Therefore, in all cases, there is assurance that cache_bytes
179+ // will be in the final range [1..FLASH_CACHE_SIZE].
120180 memcpy (dst + dst_off , fc -> cache_buf + src_off , cache_bytes );
121181
122- // cached to end
123- int copied = dst_off + cache_bytes ;
124- if ( copied < count ) fc -> read (dst + copied , addr + copied , count - copied );
182+ // Read any final bytes from flash
183+ // As noted above, dst_off represents the count of bytes read prior to the cache
184+ // while cache_bytes represents the count of bytes read from the cache;
185+ // This code block is guaranteed to overlap the cache area by at least one byte.
186+ // Thus, copied will correspond to the total bytes already copied,
187+ // and is guaranteed to be in the range [ 1 .. count ].
188+ uint32_t copied = dst_off + cache_bytes ;
189+
190+ //
191+ if ( copied < count )
192+ {
193+ fc -> read (dst + copied , addr + copied , count - copied );
194+ }
125195 }
126196 else
127197 {
198+ // not using the cache, so just forward to read from flash
128199 fc -> read (dst , addr , count );
129200 }
130201}
0 commit comments