Attention is currently required from: Simon Buhrow, Nico Huber, Thomas Heijligen, Paul Menzel, Aarya.
9 comments:
File erasure_layout.c:
Patch Set #54, Line 26: if (layout) {
rewrite your function like this:
```
if (!layout)
return;
for (unsigned int i = 0; i < erasefn_count; i++) {
free(layout[i].layout_list);
}
free(layout);
```The signature of this function should be,
```
unsigned int create_erase_layout(struct flashctx *const flashctx,
struct erase_layout **elayout)
{
```
ret is <0 on error, =0 on no erasefn_count's or >0 with no. of erasefn_count's.
Patch Set #54, Line 34: create_erase_layout
with the comments below I am expecting a few static functions that should be relatively small and this function will stitch them together.
\n
int j = 0;
while(addr < chip->total_size * 1024) {
ok `j` is shadowed below. Rewrite this while-loop construct as a `for` loop construct as:
```
for (unsigned i = 0; addr < chip->total_size * 1024; i++) {
const struct eraseblock *block = &chip->block_erasers[eraser_idx].eraseblocks[i];
block_count += block->count;
addr += block->size * block->count;
}
```
to avoid shadowing or scope to a helper function.
The other secondary message I am pealing back at is how these loops can become infinite or undefined.
trivial: `e (`
Ack
Patch Set #54, Line 62: (struct eraseblock_data *)malloc
use `calloc()`.
Is the construction of `layout[layout_idx].layout_list` on the heap it's own function? It would make digesting how deep these loops are tractable as to maintain this code.
(layout[layout_idx-1].layout_list[sub_block_index].start_addr >= start_addr &&
layout[layout_idx-1].layout_list[sub_block_index].end_addr <= end_addr &&
sub_block_index < layout[layout_idx-1].block_count)
in this new function made out of this `if`-block turn this wild predicate into a intermediate with a name.
if (layout_idx > 0) {
layout[layout_idx].layout_list[block_num].first_sub_block_index = sub_block_index;
while (layout[layout_idx-1].layout_list[sub_block_index].start_addr >= start_addr &&
layout[layout_idx-1].layout_list[sub_block_index].end_addr <= end_addr &&
sub_block_index < layout[layout_idx-1].block_count) {
sub_block_index++;
}
layout[layout_idx].layout_list[block_num].last_sub_block_index = sub_block_index - 1;
}
looks like another function with a base case of `layout_idx == 0 => return`.
To view, visit change 65879. To unsubscribe, or for help writing mail filters, visit settings.