Attention is currently required from: Aarya, Anastasia Klimchuk, Nikolai Artemiev.

View Change

8 comments:

• Commit Message:

  • Patch Set #7, Line 42:

    The progress doesn't just dump lots of stuff on the screen which
    probably won't fly because of CB:64668, but it's not hard to adjust
    this.
    

    ```suggestion
    Similar to CB:64668, an effort is made to keep the progress on a
    single line. Non-progress output is kept track of to know when
    moving to a new line cannot be avoided.
    ```

• File cli_output.c:

  • Patch Set #4, Line 119: 16 * FLASHROM_PROGRESS_NR

    Oh this is a backspace that is not deleting characters! Sorry I got so confused. […]

    Just saw that "..." is also printed below which needs to be erased as well.

• File cli_output.c:

  • Patch Set #7, Line 157: progress_printed = false;

    Those extra newlines are quite annoying. I think most of them can be deal with quite easily.

    The flag could be an enumeration:
     - progress
     - newline
     - midline
     
    This simple check should address absolute majority of prints:
    ```c
    last_line = (fmt[strlen(fmt) - 1] == '\n' ? NEWLINE : MIDLINE);
    ```

    Then both `msg_ginfo("\n");` above can be skipped if we know we're at the new line to turn
    ```
    flashrom 1.5.0-devel (git:v1.4.0-57-g83954aa8) on Linux 6.10.11 (x86_64)
    flashrom is free software, get the source code at https://flashrom.org

    Found Winbond flash chip "W25Q128.V" (16384 kB, SPI) on dummy.

    
    [READ: 100%][ERASE: 100%]...Erase/write done from 0 to ffffff
    ```
    into
    ```
    flashrom 1.5.0-devel (git:v1.4.0-57-g83954aa8) on Linux 6.10.11 (x86_64)
    flashrom is free software, get the source code at https://flashrom.org

    Found Winbond flash chip "W25Q128.V" (16384 kB, SPI) on dummy.
    [READ: 100%][ERASE: 100%]...Erase/write done from 0 to ffffff
    ```

• File erasure_layout.c:

  • Patch Set #7, Line 275: msg_cerr("ERASE FAILED!\n");

    Not added here, but this line is unreachable.

• File libflashrom.c:

  • Patch Set #7, Line 82: update_progress(flashctx, stage, 0);

    Probably worth commenting that this is used to trigger callback invocation.

• File realtek_mst_i2c_spi.c:

  • Patch Set #7, Line 400: RTK_PAGE_SIZE

    Looks like this should actually be `page_len`.

• File spi25.c:

  • Patch Set #7, Line 733: chunksize

    This should be `towrite`.

• File tests/chip.c:

  • Patch Set #7, Line 84: }

    Doesn't look like this can catch progress going backward. If you want to check for that, this could verify that stage progress is either reset to 0 or increases while total amount changes only when current value is reset. Needs creating a state in tests to keep track of stages, basically what cli code does.

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