Attention is currently required from: Simon Buhrow, Thomas Heijligen, Aarya.

Patch set 6:Code-Review +2

View Change

13 comments:

• File flashrom.c:

  • Patch Set #6, Line 1546: , erasefn_count);

    `, count_usable_erasers(flashctx));`

  • Patch Set #6, Line 1554: else

    you don't need a `else`

  • Patch Set #6, Line 1681: int ret = 0;

    if you use `ret = 1` as the default you exchange many `ret = 1`'s with one `ret = 0`.

  • Patch Set #6, Line 1683: const struct romentry *entry = NULL;

    pair with while loop construct.

  • Patch Set #6, Line 1684: struct erase_layout *erase_layout = create_erase_layout(flashctx);

    If you can help it, pair with the branch below. Don't do C89'ism where you try to put all your declarations at the top.

    ```
    	const struct flashrom_layout *const flash_layout = get_layout(flashctx);
    	struct erase_layout *erase_layout = create_erase_layout(flashctx);
    	if (!flash_layout || !erase_layout) {
    		goto _ret;
    	}
    ```

  • Patch Set #6, Line 1685: //no layout

    delete

  • Patch Set #6, Line 1690: //erase layout creation failed

    delete

  • Patch Set #6, Line 1695:

    	//not enough memory
    	if (!curcontents || !newcontents) {
    		ret = 1;
    		goto _ret;
    	}
    

    delete?

  • Patch Set #6, Line 1701: erase_write(

    line-wrap:

    ```
    erase_write(flashctx, entry->region.start,
                          entry->region.end,
                          curcontents, newcontents,
                          erase_layout, all_skipped);
    ```

  • Patch Set #6, Line 1701: (uint8_t*)

    don't cast, just use the correct type in the first place in the function definition.

  • Patch Set #6, Line 1703: !!!

    one `!` is sufficient or be consistent with the above and have none?

  • Patch Set #6, Line 1708: free_erase_layout(erase_layout, erasefn_count);

    `free_erase_layout(erase_layout, count_usable_erasers(flashctx));`

    Although I think the API of the free function should not require a length, it should be able to walk the data structure until exhaustion of the `NULL` case.

  • Patch Set #6, Line 1718: else

    ditto

To view, visit change 71173. To unsubscribe, or for help writing mail filters, visit settings.