[coreboot] [PATCH] flashrom patch for partial flash read #2
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Wed Dec 3 02:52:18 CET 2008
On 03.12.2008 02:04, Stephan GUILLOUX wrote:
> Second try for this patch, after some review/comments on #coreboot
> channel.
> - renamed generic_read() to read_flash()
> - renamed options chunk_start/size to offset/length
> - fixed some indentation spaces
> - fixed some error messages
> - replaces chk_start/size by offset/length
> - added a description in the usage()
>
> The goal for this patch is to allow flashrom to read some part of the
> flash,
> described by a pair offset/length.
> Added flashrom arguments --offset (-S) and --length (-N).
> The result is stored in a <length>-size file.
>
> The design intends to provide offset and length arguments to the
> xxx_read functions,
> so that each can read only <length> bytes from <offset>.
> Actually, for the READ part, there are still 2 or 3 functions to modify.
> If this design is agreed, I'll modify the others too.
>
> The same kind of design could be used for the VERIFY (easy), ERASE and
> WRITE (more
> difficult) parts too.
>
> Also in attachment for GMail users.
>
> Any comment is welcome.
>
> Stephan.
>
> Signed-off-by: Stephan Guilloux <mailto:stephan.guilloux at free.fr>
Partial review follows.
> +static int read_flash(struct flashchip *flash, uint8_t *buf, uint32_t offset, uint32_t length)
> +{
> + int total_size = flash->total_size * 1024;
>
total_size should be unsigned.
> +
> + if (offset >= total_size) {
> + printf ("Error: Invalid start address.\n");
> + return (1);
> + }
>
You have to check for length>= total_size as well.
> +
> + if ((length == 0) || (offset + length > total_size)) {
>
(offset + length > total_size) can overflow. This is not a
problem as long as flash chips are smaller than 2 GB and the
checks mentioned above are in place.
> + printf ("Error: Invalid size.\n");
> + return (1);
> + }
> +
> + if (flash->read == NULL) {
> + memcpy(buf, (const char *)flash->virtual_memory + offset, length);
> + } else {
> + if (flash->read(flash, buf, offset, length)) {
> + printf ("ERROR : flash read failed\n");
> + return (1);
> + }
> + }
> + return (0);
> +}
> +
> int verify_flash(struct flashchip *flash, uint8_t *buf)
> {
> int idx;
> int total_size = flash->total_size * 1024;
> uint8_t *buf2 = (uint8_t *) calloc(total_size, sizeof(char));
> - if (flash->read == NULL)
> - memcpy(buf2, (const char *)flash->virtual_memory, total_size);
> - else
> - flash->read(flash, buf2);
> +
> + if (read_flash(flash, buf2, 0, total_size)) {
> + printf ("ERROR : verification aborted.\n");
>
"ERROR : verification aborted because flash could not be read.\n"
> + return (1);
> + }
>
> printf("Verifying flash... ");
>
>
I noticed you are using parentheses around return values. That's rather
uncommon in flashrom. Please avoid them if possible.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list