Added gbb_utility (tool for Google Binary Block)

Added gbb_utility (tool for Google Binary Block)

[Randall Spangler, 2010-05-28 01:22:39]

http://codereview.chromium.org/2346001/diff/1/3
File src/platform/vboot_reference/utility/gbb_utility.cc (right):

http://codereview.chromium.org/2346001/diff/1/3#newcode147
src/platform/vboot_reference/utility/gbb_utility.cc:147: found_signatures ++;
extra whitespace

http://codereview.chromium.org/2346001/diff/1/3#newcode159
src/platform/vboot_reference/utility/gbb_utility.cc:159: // check total block
size
Check that GBB header doesn't extend past the end of the image

http://codereview.chromium.org/2346001/diff/1/3#newcode182
src/platform/vboot_reference/utility/gbb_utility.cc:182: 
Should also check that h.block_size >= GBB_HEADER_SIZE.  We could be reading an
old kernel with a new utility.

http://codereview.chromium.org/2346001/diff/1/3#newcode200
src/platform/vboot_reference/utility/gbb_utility.cc:200: // TODO(hungte) check
if the properties overlap themselves?
Yes, but can leave that for a follow-on CL.  

Should also make sure they don't overlap the GBB header itself (that is,
h.FOO_offset must be >= h.block_size)

http://codereview.chromium.org/2346001/diff/1/3#newcode201
src/platform/vboot_reference/utility/gbb_utility.cc:201: // TODO(hungte) check
if the properties exceed GBB range?
No need; we can support them being anywhere in the image.

http://codereview.chromium.org/2346001/diff/1/3#newcode462
src/platform/vboot_reference/utility/gbb_utility.cc:462:
export_property_to_file(myopts.rootkey_fn, "rootkey", util.get_rootkey());
Looks like this and update-mode always read/write all 3 properties, optionally
using different filenames

Recommend changing the way this works to something like

gbb_util --set --bmpfv=foo.fv

will set only the bitmap volume

gbb_util --get --hwid

will print only the hwid.

(also recommend using --get / -g instead of --query / -q, since -q is often used
to mean 'quiet')

http://codereview.chromium.org/2346001/diff/1/4
File src/platform/vboot_reference/utility/include/gbb_utility.h (right):

http://codereview.chromium.org/2346001/diff/1/4#newcode25
src/platform/vboot_reference/utility/include/gbb_utility.h:25: bool save_to_file
 (const char *filename);
extra whitespace

http://codereview.chromium.org/2346001/diff/1/4#newcode48
src/platform/vboot_reference/utility/include/gbb_utility.h:48: int 
search_header_signatures(const std::string &image, long *poffset) const;
extra whitespace

http://codereview.chromium.org/2346001/diff/1/5
File src/platform/vboot_reference/vboot_firmware/include/gbb_header.h (right):

http://codereview.chromium.org/2346001/diff/1/5#newcode30
src/platform/vboot_reference/vboot_firmware/include/gbb_header.h:30: uint32_t
block_size;     // GBB size in bytes
Size of the GBB header, not the entire GBB.  (So that we can handle reading a
smaller GoogleBinaryBlockHeader(), if we need a newer utility to still be able
to manipulate order files)

[Hung-Te, 2010-05-28 01:47:17]

http://codereview.chromium.org/2346001/diff/1/3
File src/platform/vboot_reference/utility/gbb_utility.cc (right):

http://codereview.chromium.org/2346001/diff/1/3#newcode462
src/platform/vboot_reference/utility/gbb_utility.cc:462:
export_property_to_file(myopts.rootkey_fn, "rootkey", util.get_rootkey());
On 2010/05/28 01:22:39, Randall Spangler wrote:
> Looks like this and update-mode always read/write all 3 properties, optionally
> using different filenames
  In fact if the file name is empty, it'll skip.
  Such coding style may cause confusion, so I'll change it.

> Recommend changing the way this works to something like
> gbb_util --set --bmpfv=foo.fv
> will set only the bitmap volume
   Yes, it was working like this way, except hardware
   ID was always reported. I'd also change that.

http://codereview.chromium.org/2346001/diff/1/5
File src/platform/vboot_reference/vboot_firmware/include/gbb_header.h (right):

http://codereview.chromium.org/2346001/diff/1/5#newcode30
src/platform/vboot_reference/vboot_firmware/include/gbb_header.h:30: uint32_t
block_size;     // GBB size in bytes
On 2010/05/28 01:22:39, Randall Spangler wrote:
> Size of the GBB header, not the entire GBB. 
  This was our question on mail and not confirmed right?
  That's why I left it not checked at first commit.
  I think I'd change this name to 'header_size'.

[Randall Spangler, 2010-05-28 16:57:34]

LGTM (looks good to me).

One suggested change for a future CL.

http://codereview.chromium.org/2346001/diff/6001/7002
File src/platform/vboot_reference/utility/gbb_utility.cc (right):

http://codereview.chromium.org/2346001/diff/6001/7002#newcode460
src/platform/vboot_reference/utility/gbb_utility.cc:460: printf("error: please
assign either get or set mode.\n");
Could assume that get is the default, and then only need --set option.