A general question: Would it make sense to update the given reference
file after a successful flash?

View Change

13 comments:

• File cli_classic.c:

  • Patch Set #2, Line 60: " --ifd read layout from an Intel Firmware Descriptor\n"

    This line is too long now for a 80-columns terminal. Though,
    I think we can just avoid to shift all these lines, see below.

  • Patch Set #2, Line 63: referencefile

    Maybe just shorten it, e.g. <ref-file>, and leave the other lines as they were.

  • Patch Set #2, Line 63: -C

    I'm not sure about the `-C` maybe only add the long option for now?

  • Patch Set #2, Line 363: contents

    "reference"

• File flashrom.c:

  • Patch Set #2, Line 2348: * @param buffer_len Size of source buffer in bytes.

    reference parameter should be mentioned here

  • Patch Set #2, Line 2355: int flashrom_image_write(struct flashctx *const flashctx, void *const buffer, const size_t buffer_len, const char *referencefile)

    I just realized that this is already part of the libflashrom
    interface which should be kept free of file i/o handling.
    You should pass another buffer here, already filled with the
    contents of the reference file, e.g. `refcontents` to align
    with `oldcontents`/`newcontents`.

  • Patch Set #2, Line 2400: */

    This justification makes sense in the commit message but doesn't
    belong here. Please condense it to a single line, e.g.

      /* If given, assume flash contains same data as `refcontents`. */

  • Patch Set #2, Line 2401:

    spurious empty line

  • Patch Set #2, Line 2402:

    	if (referencefile) {
    		msg_cinfo("Reading old flash chip contents from file... ");
    		if (read_buf_from_file(oldcontents, flash_size, referencefile)) {
    			ret = 1;
    			msg_cinfo("FAILED.\n");
    			goto _finalize_ret;
    		}
    

    move into do_write() please

  • Patch Set #2, Line 2410:

    spurious empty line

  • Patch Set #2, Line 2418:

    spurious empty line

  • Patch Set #2, Line 2438:

    	if (verify_all) {
    		msg_cerr("Checking if anything has changed.\n");
    		msg_cinfo("Reading current flash chip contents... ");
    		if (!flashctx->chip->read(flashctx, curcontents, 0, flash_size)) {
    			msg_cinfo("done.\n");
    			if (!memcmp(oldcontents, curcontents, flash_size)) {
    				nonfatal_help_message();
    				goto _finalize_ret;
    			}
    			msg_cerr("Apparently at least some data has changed.\n");
    		} else
    			msg_cerr("Can't even read anymore!\n");
    

    misaligned by accident?

  • Patch Set #2, Line 2580: goto _free_ret;

    Same as `filename`, `referencefile` should be read here if given.

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