[L] Change in flashrom[master]: flashrom.c: Add wrapper function to use the erase algorithm

[Edward O'Callaghan (Code Review)]

Attention is currently required from: Simon Buhrow, Nico Huber, Aarya, Anastasia Klimchuk.

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 )

Change subject: flashrom.c: Add wrapper function to use the erase algorithm
......................................................................

Patch Set 78:

(10 comments)

Commit Message:

https://review.coreboot.org/c/flashrom/+/66104/comment/d509bb8f_dd83febd 
PS78, Line 6: 
            : flashrom.c: Add wrapper function to use the erase algorithm
            : 
            : Add a function to call the erase algorithm.
commit message is unrelated to content.

File flashrom.c:

https://review.coreboot.org/c/flashrom/+/66104/comment/5b908617_ea80c258 
PS72, Line 1349: static int erase_by_layout
> It would be my view to more your whole implementation into its own separate file. […]

If it is easier to not have your work sitting in review forever and you want to put everything initially in flashrom.c I would suggest the following pattern for the minimal amount of reshuffling for you:

i) Create some trampoline functions that will dispatch into your algorithm and a few if branches to jump to them. Use a static bool global `use_legacy_erase_path = true;` and branch off that.
 ii) Adding all the new functions in one go atomically without the need for the prototypes hacks.
 iii) Have a small commit that toggles `use_legacy_erase_path` from `true->false`.

So for step (i), for this function in another commit rename it as `erase_by_layout_legacy()` and call it with:

```
static int erase_by_layout(struct flashctx *const flashctx)
{
     if (use_legacy_erase_path)
        return erase_by_layout_legacy(flashctx);
}
```

Subsequently for step (ii) you can do:

```
static int erase_by_layout(struct flashctx *const flashctx)
{
     if (use_legacy_erase_path)
        return erase_by_layout_legacy(flashctx);

return erase_by_layout_optimised(flashctx);
}
```

File flashrom.c:

https://review.coreboot.org/c/flashrom/+/66104/comment/f72f5d49_79174b17 
PS78, Line 720:  <= len
what is happening here? Seems like its own change but not what the commit says "add wrapping function".

https://review.coreboot.org/c/flashrom/+/66104/comment/2a6cb487_e468e330 
PS78, Line 1374: size_t
`const uint32_t`, A `size_t` is a a system specific type.

https://review.coreboot.org/c/flashrom/+/66104/comment/7e589648_a742f96f 
PS78, Line 1378: int
const

https://review.coreboot.org/c/flashrom/+/66104/comment/36afffc5_54b6d68b 
PS78, Line 1382: curcontents == NULL || newcontents == NULL
`!ptr` is canonical. Also `erase_layout` never checked.

https://review.coreboot.org/c/flashrom/+/66104/comment/a97a9573_3f4f1113 
PS78, Line 1422: write_by_layout
in a separate commit before this one, rename as `write_by_layout_legacy()` and,

```
static int write_by_layout(struct flashctx *const flashctx,
                           uint8_t *const curcontents,    
                           const uint8_t *const newcontents)
{
     if (use_legacy_erase_path)
        return write_by_layout_legacy(flashctx, curcontents, newcontents);
}
```

then you can put your new implementation in the above in this commit.

https://review.coreboot.org/c/flashrom/+/66104/comment/a59babe2_580b6c5b 
PS78, Line 1425: 	const struct flashrom_layout *const flash_layout = get_layout(flashctx);
               : 	const struct romentry *entry = NULL;
               : 	struct erase_layout *erase_layout = create_erase_layout(flashctx);
move below `erasefn_count` and `ret`

https://review.coreboot.org/c/flashrom/+/66104/comment/ad34dc3d_9c2e4f09 
PS78, Line 1427: erase_layout
check for nullarity?

https://review.coreboot.org/c/flashrom/+/66104/comment/7ba21b18_09cb3e93 
PS78, Line 1428: int 
`const`