Change in flashrom[staging]: layout: Add -i <region>[:<file>] support.

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19342 )

Change subject: layout: Add -i <region>[:<file>] support.
......................................................................

Patch Set 2:

(10 comments)

https://review.coreboot.org/#/c/19342/2/flashrom.8.tmpl
File flashrom.8.tmpl:

PS2, Line 147: <region>[:<file>]
This seems wrong. Bad merge?

PS2, Line 292: .TP
             : flashrom supports reading/writing/erasing/verifying flash chips in whole (default) or in part. To access only \
             : parts of a chip one has to use layout files and respective arguments described below.
             : .TP
             : .B "-l, --layout <layoutfile>"
             : Read layout entries from
             : .BR <layoutfile> .
             : .sp
             : Each layout entry describes an address region of the flash chip and gives it a name (hereinafter referred to as
             : a region). One entry per line is allowed with the following syntax:
             : .sp
             : .B "  startaddr:endaddr regionname"
             : .sp
             : .BR "startaddr " "and " "endaddr "
             : are hexadecimal addresses within the ROM image representing the flash ROM contents and do not refer to any
             : physical address. Please note that using a 0x prefix for those hexadecimal numbers is not necessary, but you
             : can't specify decimal/octal numbers.
             : .BR "regionname " "is the name for the region from " "startaddr " "to " "endaddr " "(both addresses included)."
             : Spaces, tabs, newlines, colons, single or double quotes are not allowed in region names.
             : .sp
             : Example content of file rom.layout:
             : .sp
             :   0x00000000:0x00008fff gfx_rom
             :   0x00009000:0x0003ffff normal
             :   0x00040000:0x0007ffff fallback
             : .sp
             : .TP
             : .B "-i, --include <name>[:<regionfile>]"
             : Work on the flash region
             : .B name
             : instead of the full address space if a layout file is given and parsed correctly.
             : Multiple such include parameters can be used to work on the union of different regions.
             : .sp
             : The optional
             : .B regionfile
             : parameter specifies the name of a file that is used to map the contents of that very region only.
             : The optional file parameter causes the contents of this region to be replaced by the contents of the file
             : specified here.
             : .sp
             : If you only want to update the regions named
             : .BR "normal " "and " "gfx_rom " "in a ROM based on the layout mentioned above, run"
             : .sp
             : .B "  flashrom -p prog -l rom.layout -i normal -i "gfx_rom" -w some.rom"
             : .sp
             : Overlapping regions are resolved in an implementation-dependent manner (or may even yield an error) - do
             : .BR "not " "rely on it."
             : .sp
Should be merged with the old description above. Probably just a bad merge.

https://review.coreboot.org/#/c/19342/2/flashrom.c
File flashrom.c:

PS2, Line 1308: 		msg_gerr("Error: Image size (%jd B) doesn't match the flash chip's size (%lu B)!\n",
              : 			 (intmax_t)image_stat.st_size, size);
remove

Line 1353: 
IMHO, not having an empty line here was intentional.

Line 1392: static const struct fl_layout *get_layout(const struct flashctx *const flashctx);
If we make it the standard, the declaration should probably move
to `layout.h`.

Line 1404:  * @param size     Size of chip-sized buffer
Now that we pass the flashctx, the size argument is redundant.
We should either drop it, or bail out if they don't match.

Line 1407:  *	   1 if all available erase functions failed.
ahem

PS2, Line 1427: *
*const ? to match the other "variables"

Line 1444: 
double blank line?

https://review.coreboot.org/#/c/19342/2/layout.h
File layout.h:

PS2, Line 43: char
`const char`? we'd never want to change it, would we?

If we're going to have something dynamically allocated here,
we should also reevaluate how `name` is handled.