Attention is currently required from: Nico Huber, Angel Pons, Daniel Campello.

Patch set 1:Code-Review +2

View Change

4 comments:

• File cli_classic.c:

  • Patch Set #1, Line 140: static char *get_optional_filename(char *argv[])

    Can you leave a comment on this function what precisely it is expected to do here. Longer term as the flashrom team here works on improving unit-test coverage I guess things like this are nice candidates for folks to get started with. Therefore comments help them form the right tests.

    I left some comments in flashrom.c with the diff with how the chromiumos tree behaves with the file descriptors when the user doesn't specify them. I was wondering if you can speak to that diff more so that we can close the gap off there once this lands?

• File flashrom.c:

  • Patch Set #1, Line 1351: if ((image = fopen(filename, "rb")) == NULL) {

    ```

    • if (!strncmp(filename, "-", sizeof("-")))
    • image = fdopen(STDIN_FILENO, "rb");
    • else
    • image = fopen(filename, "rb");
    • if (image == NULL) {
    • + if ((image = fopen(filename, "rb")) == NULL) {
    • ```

  • Patch Set #1, Line 1361:

    
    	if (image_stat.st_size != (intmax_t)size) {
    

    ```

    • if ((image_stat.st_size != (__off_t)size) &&
    • (strncmp(filename, "-", sizeof("-")))) {
    • + if (image_stat.st_size != (intmax_t)size) {
    • ```

  • Patch Set #1, Line 1439: if ((image = fopen(filename, "wb")) == NULL) {

    ```

    • if (!strncmp(filename, "-", sizeof("-")))
    • image = fdopen(STDOUT_FILENO, "wb");
    • else
    • image = fopen(filename, "wb");
    • if (image == NULL) {
    • + if ((image = fopen(filename, "wb")) == NULL) {
    • ```

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