From 8ba3857ab2388b354886561a054e3abdb73e0d86 Mon Sep 17 00:00:00 2001 From: David Hendricks Date: Mon, 11 Mar 2013 15:55:57 -0700 Subject: [PATCH 01/16] add PRESUBMIT.cfg This adds a presubmit config file so we don't get annoying repo upload warnings for tabs and software license. BUG=none BRANCH=none TEST=tried uploading a change, didn't get nagged for tabs and license Change-Id: Ia46ae58c6281fb6cc7ac0864d45c613908cb2df4 Reviewed-on: https://gerrit.chromium.org/gerrit/45141 Reviewed-by: Duncan Laurie Commit-Queue: David Hendricks Tested-by: David Hendricks --- PRESUBMIT.cfg | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 PRESUBMIT.cfg diff --git a/PRESUBMIT.cfg b/PRESUBMIT.cfg new file mode 100644 index 0000000..420337e --- /dev/null +++ b/PRESUBMIT.cfg @@ -0,0 +1,5 @@ +# This project uses BSD/GPLv2 for the license and tabs for indentation. + +[Hook Overrides] +cros_license_check: false +tab_check: false From 60415d5eb09d9c9f31e14a04fa00eceb948dd1a9 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 18 Apr 2017 09:50:57 +0800 Subject: [PATCH 02/16] fmap_find_area: Use const pointers for fmap and return value fmap_find_area does not modify the fmap, let's use const pointers. BUG=none TEST=emerge flashmap Change-Id: Ie5c64c3df3b2ef44a2e2ca0dbef4dd3403d91d75 Reviewed-on: https://chromium-review.googlesource.com/479037 Commit-Ready: Nicolas Boichat Tested-by: Nicolas Boichat Reviewed-by: Randall Spangler --- lib/fmap.c | 9 +++++---- lib/fmap.h | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/fmap.c b/lib/fmap.c index 5022434..8984697 100644 --- a/lib/fmap.c +++ b/lib/fmap.c @@ -349,10 +349,11 @@ int fmap_append_area(struct fmap **fmap, return new_size; } -struct fmap_area *fmap_find_area(struct fmap *fmap, const char *name) +const struct fmap_area *fmap_find_area(const struct fmap *fmap, + const char *name) { int i; - struct fmap_area *area = NULL; + const struct fmap_area *area = NULL; if (!fmap || !name) return NULL; @@ -532,10 +533,10 @@ static int fmap_append_area_test(struct fmap **fmap) return status; } -static int fmap_find_area_test(struct fmap *fmap) +static int fmap_find_area_test(const struct fmap *fmap) { status = fail; - char area_name[] = "test_area_1"; + const char area_name[] = "test_area_1"; if (fmap_find_area(NULL, area_name) || fmap_find_area(fmap, NULL)) { diff --git a/lib/fmap.h b/lib/fmap.h index 9cea22d..d5809dd 100644 --- a/lib/fmap.h +++ b/lib/fmap.h @@ -184,7 +184,8 @@ extern int fmap_append_area(struct fmap **fmap, * returns a pointer to the entry in the fmap structure if successful * returns NULL to indicate failure or if no matching area entry is found */ -extern struct fmap_area *fmap_find_area(struct fmap *fmap, const char *name); +extern const struct fmap_area *fmap_find_area(const struct fmap *fmap, + const char *name); /* unit testing stuff */ extern int fmap_test(); From 21831b3583f9dc29607721468b20770a80d0c147 Mon Sep 17 00:00:00 2001 From: Chih-Yu Huang Date: Tue, 16 May 2017 18:57:08 +0800 Subject: [PATCH 03/16] fmap.h: Add `extern "C"` to make it compatible to C++. fmap.h is a C header file. We add `extern "C"` to let C++ code be able to include it. BUG=none TEST=emerge flashmap successfully build a c++ program which includes fmap.h Change-Id: I0317ddc9ce7182d69899d7756688ee7dcaded26a Reviewed-on: https://chromium-review.googlesource.com/505677 Commit-Ready: Nicolas Boichat Tested-by: Chih-Yu Huang Reviewed-by: Mike Frysinger --- lib/fmap.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/fmap.h b/lib/fmap.h index d5809dd..51bd768 100644 --- a/lib/fmap.h +++ b/lib/fmap.h @@ -36,6 +36,10 @@ #ifndef FLASHMAP_LIB_FMAP_H__ #define FLASHMAP_LIB_FMAP_H__ +#ifdef __cplusplus +extern "C" { +#endif + #include #include @@ -190,4 +194,8 @@ extern const struct fmap_area *fmap_find_area(const struct fmap *fmap, /* unit testing stuff */ extern int fmap_test(); +#ifdef __cplusplus +} /* extern "C" */ +#endif + #endif /* FLASHMAP_LIB_FMAP_H__*/ From 0a24e5b75dc87d385e5fa2411b0690a3ae9c1fd2 Mon Sep 17 00:00:00 2001 From: Drew Davenport Date: Thu, 25 May 2017 14:13:55 -0600 Subject: [PATCH 04/16] flashmap: add fmap_unittest.py - Remove test functionality from fmap.py's main function and replace with appropriate unit tests in fmap_unittest.py - Bring over some formatting and lint fixes from factory/setup/fmap.py - Add *.pyc to .gitignore BUG=chromium:726356 TEST=ran unit tests Change-Id: I4cb99efa3d924483dfdff9be54f6c3fc10040f3f Reviewed-on: https://chromium-review.googlesource.com/516242 Commit-Ready: Drew Davenport Tested-by: Drew Davenport Reviewed-by: Hung-Te Lin --- .gitignore | 1 + fmap.py | 36 ++++++++++++++++---------- fmap_unittest.py | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 fmap_unittest.py diff --git a/.gitignore b/.gitignore index 7fee136..f95d7e1 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ *.cmd *.gcda *.gcno +*.pyc .config .config.old scripts/basic/docproc diff --git a/fmap.py b/fmap.py index 3f26b54..21519a4 100644 --- a/fmap.py +++ b/fmap.py @@ -51,11 +51,13 @@ tuple of decoded area flags. """ + import struct import sys + # constants imported from lib/fmap.h -FMAP_SIGNATURE = "__FMAP__" +FMAP_SIGNATURE = '__FMAP__' FMAP_VER_MAJOR = 1 FMAP_VER_MINOR = 0 FMAP_STRLEN = 32 @@ -82,9 +84,10 @@ 'flags', ) + # format string -FMAP_HEADER_FORMAT = "<8sBBQI%dsH" % (FMAP_STRLEN) -FMAP_AREA_FORMAT = "' + sys.exit(1) + + filename = sys.argv[1] + print 'Decoding FMAP from: %s' % filename + blob = open(filename).read() obj = fmap_decode(blob) print obj - blob2 = fmap_encode(obj) - obj2 = fmap_decode(blob2) - print obj2 - assert obj == obj2 + +if __name__ == '__main__': + main() diff --git a/fmap_unittest.py b/fmap_unittest.py new file mode 100644 index 0000000..7006520 --- /dev/null +++ b/fmap_unittest.py @@ -0,0 +1,67 @@ +#!/usr/bin/python +# +# Copyright 2017 The Chromium OS Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. +"""Unit test for fmap module.""" + +import unittest + +import fmap + +# Expected decoded fmap structure from bin/example.bin +_EXAMPLE_BIN_FMAP = { + 'ver_major': 1, + 'ver_minor': 0, + 'name': 'example', + 'nareas': 4, + 'base': 0, + 'signature': '__FMAP__', + 'areas': [{ + 'FLAGS': ('FMAP_AREA_STATIC',), + 'size': 128, + 'flags': 1, + 'name': 'bootblock', + 'offset': 0 + }, { + 'FLAGS': ('FMAP_AREA_COMPRESSED', 'FMAP_AREA_STATIC'), + 'size': 128, + 'flags': 3, + 'name': 'normal', + 'offset': 128 + }, { + 'FLAGS': ('FMAP_AREA_COMPRESSED', 'FMAP_AREA_STATIC'), + 'size': 256, + 'flags': 3, + 'name': 'fallback', + 'offset': 256 + }, { + 'FLAGS': (), + 'size': 512, + 'flags': 0, + 'name': 'data', + 'offset': 512 + }], + 'size': 1024 +} + + +class FmapTest(unittest.TestCase): + """Unit test for fmap module.""" + + def setUp(self): + with open('bin/example.bin') as f: + self.example_blob = f.read() + + def testDecode(self): + decoded = fmap.fmap_decode(self.example_blob) + self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + + def testEncode(self): + encoded = fmap.fmap_encode(_EXAMPLE_BIN_FMAP) + # example.bin contains other binary data besides the fmap + self.assertIn(encoded, self.example_blob) + + +if __name__ == '__main__': + unittest.main() From 3400d55f7bb96c8b5b76221672585878ca931e12 Mon Sep 17 00:00:00 2001 From: Drew Davenport Date: Thu, 25 May 2017 14:45:58 -0600 Subject: [PATCH 05/16] flashmap: Bring over changes from factory repo fmap.py is identical to the one in factory/setup/fmap.py except: - no test functionality in main function - module docstring and license Added some unit tests for the new functionality BUG=chromium:726356 TEST=ran unittests Change-Id: Ib67bd4260cf4e1cc08543b822521a4adcf0cbce6 Reviewed-on: https://chromium-review.googlesource.com/516382 Commit-Ready: Drew Davenport Tested-by: Drew Davenport Reviewed-by: Drew Davenport --- fmap.py | 70 +++++++++++++++++++++++++++++++++++++++++++----- fmap_unittest.py | 25 +++++++++++++++++ 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/fmap.py b/fmap.py index 21519a4..830a64e 100644 --- a/fmap.py +++ b/fmap.py @@ -52,6 +52,7 @@ """ +import logging import struct import sys @@ -59,8 +60,10 @@ # constants imported from lib/fmap.h FMAP_SIGNATURE = '__FMAP__' FMAP_VER_MAJOR = 1 -FMAP_VER_MINOR = 0 +FMAP_VER_MINOR_MIN = 0 +FMAP_VER_MINOR_MAX = 1 FMAP_STRLEN = 32 +FMAP_SEARCH_STRIDE = 4 FMAP_FLAGS = { 'FMAP_AREA_STATIC': 1 << 0, @@ -102,7 +105,8 @@ def _fmap_decode_header(blob, offset): if header['signature'] != FMAP_SIGNATURE: raise struct.error('Invalid signature') if (header['ver_major'] != FMAP_VER_MAJOR or - header['ver_minor'] != FMAP_VER_MINOR): + header['ver_minor'] < FMAP_VER_MINOR_MIN or + header['ver_minor'] > FMAP_VER_MINOR_MAX): raise struct.error('Incompatible version') # convert null-terminated names @@ -128,7 +132,58 @@ def _fmap_decode_area_flags(area_flags): return tuple([name for name in FMAP_FLAGS if area_flags & FMAP_FLAGS[name]]) -def fmap_decode(blob, offset=None): +def _fmap_check_name(fmap, name): + """Checks if the FMAP structure has correct name. + + Args: + fmap: A decoded FMAP structure. + name: A string to specify expected FMAP name. + + Raises: + struct.error if the name does not match. + """ + if fmap['name'] != name: + raise struct.error('Incorrect FMAP (found: "%s", expected: "%s")' % + (fmap['name'], name)) + + +def _fmap_search_header(blob, fmap_name=None): + """Searches FMAP headers in given blob. + + Uses same logic from vboot_reference/host/lib/fmap.c. + + Args: + blob: A string containing FMAP data. + fmap_name: A string to specify target FMAP name. + + Returns: + A tuple of (fmap, size, offset). + """ + lim = len(blob) - struct.calcsize(FMAP_HEADER_FORMAT) + align = FMAP_SEARCH_STRIDE + + # Search large alignments before small ones to find "right" FMAP. + while align <= lim: + align *= 2 + + while align >= FMAP_SEARCH_STRIDE: + for offset in xrange(align, lim + 1, align * 2): + if not blob.startswith(FMAP_SIGNATURE, offset): + continue + try: + (fmap, size) = _fmap_decode_header(blob, offset) + if fmap_name is not None: + _fmap_check_name(fmap, fmap_name) + return (fmap, size, offset) + except struct.error as e: + # Search for next FMAP candidate. + logging.debug('Continue searching FMAP due to exception %r', e) + pass + align /= 2 + raise struct.error('No valid FMAP signatures.') + + +def fmap_decode(blob, offset=None, fmap_name=None): """ Decodes a blob to FMAP dictionary object. Arguments: @@ -137,10 +192,13 @@ def fmap_decode(blob, offset=None): the blob. """ fmap = {} + if offset is None: - # try search magic in fmap - offset = blob.find(FMAP_SIGNATURE) - (fmap, size) = _fmap_decode_header(blob, offset) + (fmap, size, offset) = _fmap_search_header(blob, fmap_name) + else: + (fmap, size) = _fmap_decode_header(blob, offset) + if fmap_name is not None: + _fmap_check_name(fmap, fmap_name) fmap['areas'] = [] offset = offset + size for _ in range(fmap['nareas']): diff --git a/fmap_unittest.py b/fmap_unittest.py index 7006520..4473da9 100644 --- a/fmap_unittest.py +++ b/fmap_unittest.py @@ -5,6 +5,7 @@ # found in the LICENSE file. """Unit test for fmap module.""" +import struct import unittest import fmap @@ -57,6 +58,30 @@ def testDecode(self): decoded = fmap.fmap_decode(self.example_blob) self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + def testDecodeWithOffset(self): + decoded = fmap.fmap_decode(self.example_blob, 512) + self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + + def testDecodeWithName(self): + decoded = fmap.fmap_decode(self.example_blob, fmap_name='example') + self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + decoded = fmap.fmap_decode(self.example_blob, 512, 'example') + self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + + def testDecodeWithWrongName(self): + with self.assertRaises(struct.error): + decoded = fmap.fmap_decode(self.example_blob, fmap_name='banana') + with self.assertRaises(struct.error): + decoded = fmap.fmap_decode(self.example_blob, 512, 'banana') + + def testDecodeWithOffset(self): + decoded = fmap.fmap_decode(self.example_blob, 512) + self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + + def testDecodeWithWrongOffset(self): + with self.assertRaises(struct.error): + fmap.fmap_decode(self.example_blob, 42) + def testEncode(self): encoded = fmap.fmap_encode(_EXAMPLE_BIN_FMAP) # example.bin contains other binary data besides the fmap From c02e213fd8b267715d180199cb14f9424c0695fe Mon Sep 17 00:00:00 2001 From: Hung-Te Lin Date: Thu, 28 Feb 2019 17:22:56 +0800 Subject: [PATCH 06/16] flashmap: Add FMAP_AREA_PRESERVE in area flags Many projects started their initial builds without knowing that some sections must be preserved when being updated. This may be solved by adding section name to 'preserved' list in firmware updater (for instance, CL:1239797), or include that section as sub area of {RO,RW}_PRESERVE. However, there are problems in both solution. For example, installing an older image will run old updater, which will not preserve the new names. Also, if there are multiple sections must be preserved (and not contiguous - see CL:1493629) there will be problems. Additionally, changing FMAP layout usually causes more problems. As a result, adding the description in FMAP area would be the better idea. The new FMAP_AREA_PRESERVE is a suggestion for updater to preserve this section if possible. Programs constructing FMAP must define its own way to add such attribute. BUG=b:126637087,chromium:936768 TEST=sudo emerge flashmap Change-Id: I61e80aa13efe36177b76b5a9d8d3a4b8bcbd9bac Reviewed-on: https://chromium-review.googlesource.com/1493767 Commit-Ready: ChromeOS CL Exonerator Bot Tested-by: Hung-Te Lin Reviewed-by: Duncan Laurie --- fmap.py | 2 ++ lib/fmap.c | 1 + lib/fmap.h | 1 + 3 files changed, 4 insertions(+) diff --git a/fmap.py b/fmap.py index 830a64e..982d1a5 100644 --- a/fmap.py +++ b/fmap.py @@ -68,6 +68,8 @@ FMAP_FLAGS = { 'FMAP_AREA_STATIC': 1 << 0, 'FMAP_AREA_COMPRESSED': 1 << 1, + 'FMAP_AREA_RO': 1 << 2, + 'FMAP_AREA_PRESERVE': 1 << 3, } FMAP_HEADER_NAMES = ( diff --git a/lib/fmap.c b/lib/fmap.c index 8984697..c62735d 100644 --- a/lib/fmap.c +++ b/lib/fmap.c @@ -57,6 +57,7 @@ const struct valstr flag_lut[] = { { FMAP_AREA_STATIC, "static" }, { FMAP_AREA_COMPRESSED, "compressed" }, { FMAP_AREA_RO, "ro" }, + { FMAP_AREA_PRESERVE, "preserve" }, }; /* returns size of fmap data structure if successful, <0 to indicate error */ diff --git a/lib/fmap.h b/lib/fmap.h index 51bd768..05993fd 100644 --- a/lib/fmap.h +++ b/lib/fmap.h @@ -54,6 +54,7 @@ enum fmap_flags { FMAP_AREA_STATIC = 1 << 0, FMAP_AREA_COMPRESSED = 1 << 1, FMAP_AREA_RO = 1 << 2, + FMAP_AREA_PRESERVE = 1 << 3, /* Should be preserved on update. */ }; /* Mapping of volatile and static regions in firmware binary */ From fb0dbd35009b9642efc26df77e8112525c2e645f Mon Sep 17 00:00:00 2001 From: Kirtika Ruchandani Date: Sat, 25 May 2019 12:52:31 -0700 Subject: [PATCH 07/16] flashmap: Update OWNERS Remove folks not on project/at Google. BUG=None TEST=None Change-Id: I6f4c124724a113b542427548d373877d07f05544 Signed-off-by: Kirtika Ruchandani Reviewed-on: https://chromium-review.googlesource.com/1630647 Commit-Ready: Kirtika Ruchandani Tested-by: Kirtika Ruchandani Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- OWNERS | 1 - 1 file changed, 1 deletion(-) diff --git a/OWNERS b/OWNERS index e7ca0e2..c4a566a 100644 --- a/OWNERS +++ b/OWNERS @@ -1,2 +1 @@ -dhendrix@chromium.org dlaurie@chromium.org From 9f7c466b92db25e1f620c0d1f6dd03f513011edf Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Wed, 24 Jul 2019 18:02:37 -0400 Subject: [PATCH 08/16] OWNERS: add @google.com too BUG=None TEST=None Exempt-From-Owner-Approval: updating OWNERS itself w/no existing approvers Change-Id: If529419e8004dc69dfd000b03d8ed830d61f48d9 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashmap/+/1716019 Tested-by: Mike Frysinger Reviewed-by: Duncan Laurie --- OWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/OWNERS b/OWNERS index c4a566a..e525dff 100644 --- a/OWNERS +++ b/OWNERS @@ -1 +1,2 @@ dlaurie@chromium.org +dlaurie@google.com From 909d33096042b6f61b6e920b8e5c3cf99b222914 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 9 Jul 2019 13:54:46 -0400 Subject: [PATCH 09/16] make python scripts executable These are expected to be run directly, so mark both of them executable so it's easier to run tests. BUG=chromium:982465 TEST=running both directly works Change-Id: Ib76c3d61a80c554ad03ed68065f9afa2c6ccb887 Reviewed-on: https://chromium-review.googlesource.com/1693881 Tested-by: Mike Frysinger Commit-Ready: ChromeOS CL Exonerator Bot Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- fmap.py | 0 fmap_unittest.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 fmap.py mode change 100644 => 100755 fmap_unittest.py diff --git a/fmap.py b/fmap.py old mode 100644 new mode 100755 diff --git a/fmap_unittest.py b/fmap_unittest.py old mode 100644 new mode 100755 From 1076e97ba39a7a28750ead1599166a35d2218a62 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 9 Jul 2019 13:54:09 -0400 Subject: [PATCH 10/16] PRESUBMIT: run fmap unittests These are fast so shouldn't be a problem to run. BUG=chromium:982465 TEST=`repo upload` runs these unittests Change-Id: Ib32e56e28ed0b349370d46553a032e99ff92791c Reviewed-on: https://chromium-review.googlesource.com/1693882 Tested-by: Mike Frysinger Commit-Ready: ChromeOS CL Exonerator Bot Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- PRESUBMIT.cfg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/PRESUBMIT.cfg b/PRESUBMIT.cfg index 420337e..8238211 100644 --- a/PRESUBMIT.cfg +++ b/PRESUBMIT.cfg @@ -1,5 +1,8 @@ # This project uses BSD/GPLv2 for the license and tabs for indentation. +[Hook Scripts] +fmap_unittest py2 = python2 ./fmap_unittest.py + [Hook Overrides] cros_license_check: false tab_check: false From 43fd36bd5484e78f602bb870bb160debb2890396 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 9 Jul 2019 13:58:10 -0400 Subject: [PATCH 11/16] fmap.py: use argparse for parsing the command line This makes the code easier to manage and provides automatic support for things like -h/--help. BUG=chromium:982465 TEST=`./fmap.py bin/example.bin` works Change-Id: I7a45d414cb2ed2d23bf65978b9dd06e05fc99cd7 Reviewed-on: https://chromium-review.googlesource.com/1693883 Tested-by: Mike Frysinger Commit-Ready: Mike Frysinger Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- fmap.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/fmap.py b/fmap.py index 982d1a5..a3ae343 100755 --- a/fmap.py +++ b/fmap.py @@ -33,9 +33,7 @@ # GNU General Public License ("GPL") version 2 as published by the Free # Software Foundation. -""" -This module provides basic encode and decode functionality to the flashrom -memory map (FMAP) structure. +"""Basic encode/decode functionality of flashrom memory map (FMAP) structures. Usage: (decode) @@ -52,6 +50,7 @@ """ +import argparse import logging import struct import sys @@ -237,18 +236,25 @@ def fmap_encode(obj): return blob -def main(): +def get_parser(): + """Return a command line parser.""" + parser = argparse.ArgumentParser( + description=__doc__, + formatter_class=argparse.RawTextHelpFormatter) + parser.add_argument('file', help='The file to decode & print.') + return parser + + +def main(argv): """Decode FMAP from supplied file and print.""" - if len(sys.argv) < 2: - print 'Usage: fmap.py ' - sys.exit(1) + parser = get_parser() + opts = parser.parse_args(argv) - filename = sys.argv[1] - print 'Decoding FMAP from: %s' % filename - blob = open(filename).read() + print('Decoding FMAP from: %s' % opts.file) + blob = open(opts.file, 'rb').read() obj = fmap_decode(blob) print obj if __name__ == '__main__': - main() + sys.exit(main(sys.argv[1:])) From 0cfd3b4222c65cfbe25f1cb07def62786ed31da8 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 9 Jul 2019 14:03:12 -0400 Subject: [PATCH 12/16] fmap.py: pretty print the object by default Dumping the structure as a single line makes it hard to read. Format it for use humans by default and add a --raw option for people who are using this tool in scripts. BUG=chromium:982465 TEST=`./fmap.py bin/example.bin` shows human formatted output TEST=`./fmap.py --raw bin/example.bin` shows one line Change-Id: I3232cdf104ae6edfbbc03e848166455710653f9d Reviewed-on: https://chromium-review.googlesource.com/1693884 Tested-by: Mike Frysinger Commit-Ready: Mike Frysinger Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- fmap.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/fmap.py b/fmap.py index a3ae343..9b635a1 100755 --- a/fmap.py +++ b/fmap.py @@ -52,6 +52,7 @@ import argparse import logging +import pprint import struct import sys @@ -242,6 +243,8 @@ def get_parser(): description=__doc__, formatter_class=argparse.RawTextHelpFormatter) parser.add_argument('file', help='The file to decode & print.') + parser.add_argument('--raw', action='store_true', + help='Dump the object output for scripts.') return parser @@ -250,10 +253,15 @@ def main(argv): parser = get_parser() opts = parser.parse_args(argv) - print('Decoding FMAP from: %s' % opts.file) + if not opts.raw: + print('Decoding FMAP from: %s' % opts.file) blob = open(opts.file, 'rb').read() obj = fmap_decode(blob) - print obj + if opts.raw: + print(obj) + else: + pp = pprint.PrettyPrinter(indent=2) + pp.pprint(obj) if __name__ == '__main__': From 6d45420633977d5d5a6cce586a427ea04aaefd18 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 9 Jul 2019 18:09:20 -0400 Subject: [PATCH 13/16] fmap.py: fix docstring style issues `cros lint` flags a bunch of issues with these docstrings. Clean up the style to pass all the checks. BUG=chromium:982465 TEST=`cros lint` on these no longer complains about docstrings Change-Id: I7caf0275ce1441bb4031f7d8e9e0ac96bfdf9b05 Reviewed-on: https://chromium-review.googlesource.com/1693885 Tested-by: Mike Frysinger Commit-Ready: Mike Frysinger Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- fmap.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/fmap.py b/fmap.py index 9b635a1..936b1ce 100755 --- a/fmap.py +++ b/fmap.py @@ -96,7 +96,7 @@ def _fmap_decode_header(blob, offset): - """ (internal) Decodes a FMAP header from blob by offset""" + """(internal) Decodes a FMAP header from blob by offset""" header = {} for (name, value) in zip(FMAP_HEADER_NAMES, struct.unpack_from(FMAP_HEADER_FORMAT, @@ -117,7 +117,7 @@ def _fmap_decode_header(blob, offset): def _fmap_decode_area(blob, offset): - """ (internal) Decodes a FMAP area record from blob by offset """ + """(internal) Decodes a FMAP area record from blob by offset""" area = {} for (name, value) in zip(FMAP_AREA_NAMES, struct.unpack_from(FMAP_AREA_FORMAT, blob, offset)): @@ -130,7 +130,7 @@ def _fmap_decode_area(blob, offset): def _fmap_decode_area_flags(area_flags): - """ (internal) Decodes a FMAP flags property """ + """(internal) Decodes a FMAP flags property""" return tuple([name for name in FMAP_FLAGS if area_flags & FMAP_FLAGS[name]]) @@ -180,18 +180,18 @@ def _fmap_search_header(blob, fmap_name=None): except struct.error as e: # Search for next FMAP candidate. logging.debug('Continue searching FMAP due to exception %r', e) - pass - align /= 2 + align //= 2 raise struct.error('No valid FMAP signatures.') def fmap_decode(blob, offset=None, fmap_name=None): - """ Decodes a blob to FMAP dictionary object. + """Decodes a blob to FMAP dictionary object. - Arguments: + Args: blob: a binary data containing FMAP structure. offset: starting offset of FMAP. When omitted, fmap_decode will search in the blob. + fmap_name: A string to specify target FMAP name. """ fmap = {} @@ -211,21 +211,21 @@ def fmap_decode(blob, offset=None, fmap_name=None): def _fmap_encode_header(obj): - """ (internal) Encodes a FMAP header """ + """(internal) Encodes a FMAP header""" values = [obj[name] for name in FMAP_HEADER_NAMES] return struct.pack(FMAP_HEADER_FORMAT, *values) def _fmap_encode_area(obj): - """ (internal) Encodes a FMAP area entry """ + """(internal) Encodes a FMAP area entry""" values = [obj[name] for name in FMAP_AREA_NAMES] return struct.pack(FMAP_AREA_FORMAT, *values) def fmap_encode(obj): - """ Encodes a FMAP dictionary object to blob. + """Encodes a FMAP dictionary object to blob. - Arguments + Args: obj: a FMAP dictionary object. """ # fix up values From 8d4c2f7aa209432730206c3941d2c7a67ef05090 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 9 Jul 2019 18:13:18 -0400 Subject: [PATCH 14/16] fmap_unittest.py: clean up lint warnings The assertEquals helper is deprecated in favor of assertEqual, so rename all the users to that. The testDecodeWithWrongName defines a decoded variable but doesn't do anything other than assign to it. We don't need that for this particular test (which is checking for assertions), so drop them. The testDecodeWithOffset function was duplicated (exact same name & test code), so delete one of the duplicates. BUG=chromium:982465 TEST=`cros lint` on this no longer complains TEST=`./fmap_unittest.py` still passes Change-Id: I59c9b3a9b95283ecae9948c6b7e13dd86d54a4c2 Reviewed-on: https://chromium-review.googlesource.com/1693886 Tested-by: Mike Frysinger Commit-Ready: Mike Frysinger Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- fmap_unittest.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/fmap_unittest.py b/fmap_unittest.py index 4473da9..4a5bffa 100755 --- a/fmap_unittest.py +++ b/fmap_unittest.py @@ -50,33 +50,32 @@ class FmapTest(unittest.TestCase): """Unit test for fmap module.""" + # All failures to diff the entire struct. + maxDiff = None + def setUp(self): with open('bin/example.bin') as f: self.example_blob = f.read() def testDecode(self): decoded = fmap.fmap_decode(self.example_blob) - self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + self.assertEqual(_EXAMPLE_BIN_FMAP, decoded) def testDecodeWithOffset(self): decoded = fmap.fmap_decode(self.example_blob, 512) - self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + self.assertEqual(_EXAMPLE_BIN_FMAP, decoded) def testDecodeWithName(self): decoded = fmap.fmap_decode(self.example_blob, fmap_name='example') - self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + self.assertEqual(_EXAMPLE_BIN_FMAP, decoded) decoded = fmap.fmap_decode(self.example_blob, 512, 'example') - self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + self.assertEqual(_EXAMPLE_BIN_FMAP, decoded) def testDecodeWithWrongName(self): with self.assertRaises(struct.error): - decoded = fmap.fmap_decode(self.example_blob, fmap_name='banana') + fmap.fmap_decode(self.example_blob, fmap_name='banana') with self.assertRaises(struct.error): - decoded = fmap.fmap_decode(self.example_blob, 512, 'banana') - - def testDecodeWithOffset(self): - decoded = fmap.fmap_decode(self.example_blob, 512) - self.assertEquals(_EXAMPLE_BIN_FMAP, decoded) + fmap.fmap_decode(self.example_blob, 512, 'banana') def testDecodeWithWrongOffset(self): with self.assertRaises(struct.error): From 906cb98ced72631dd3a777a59dad84d352ec89b4 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 9 Jul 2019 18:15:42 -0400 Subject: [PATCH 15/16] fmap.py: sort the flags tuple The output of this function is arbitrary because it's based on the ordering of the dict keys of FMAP_FLAGS. This leads to the unittest sometimes passing & sometimes failing. Since this is meant for us humans, sort the tuple so it's stable and unittests are reliable. BUG=chromium:982465 TEST=`./fmap.py bin/example.bin` works Change-Id: Ida9e6768aa6662a400655c52bc2ba793197ab591 Reviewed-on: https://chromium-review.googlesource.com/1693887 Tested-by: Mike Frysinger Commit-Ready: Mike Frysinger Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- fmap.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fmap.py b/fmap.py index 936b1ce..a5ec8ef 100755 --- a/fmap.py +++ b/fmap.py @@ -131,7 +131,9 @@ def _fmap_decode_area(blob, offset): def _fmap_decode_area_flags(area_flags): """(internal) Decodes a FMAP flags property""" - return tuple([name for name in FMAP_FLAGS if area_flags & FMAP_FLAGS[name]]) + # Since FMAP_FLAGS is a dict with arbitrary ordering, sort the list so the + # output is stable. Also sorting is nicer for humans. + return tuple(sorted(x for x in FMAP_FLAGS if area_flags & FMAP_FLAGS[x])) def _fmap_check_name(fmap, name): From 55f1ea65b268ff4b16f3463f1b899ef205ba03cf Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 9 Jul 2019 18:31:50 -0400 Subject: [PATCH 16/16] rework python scripts to support Python 2 & 3 We run the unittests in both python versions to maintain coverage, and we run things through `cros lint` during upload to help prevent regressions. We also switch the default to Python 3 to keep people on their toes. BUG=chromium:982465 TEST=running unittests against python2 & python3 pass TEST=`./fmap.py bin/example.bin` through python2 & python3 works Change-Id: I79f0ce59664ea42dfa18e45ccde0dffc0c18227d Reviewed-on: https://chromium-review.googlesource.com/1693888 Tested-by: Mike Frysinger Commit-Ready: Mike Frysinger Legacy-Commit-Queue: Commit Bot Reviewed-by: Duncan Laurie --- PRESUBMIT.cfg | 2 ++ fmap.py | 40 +++++++++++++++++++++++++++++++++++----- fmap_unittest.py | 8 ++++++-- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/PRESUBMIT.cfg b/PRESUBMIT.cfg index 8238211..2150455 100644 --- a/PRESUBMIT.cfg +++ b/PRESUBMIT.cfg @@ -2,6 +2,8 @@ [Hook Scripts] fmap_unittest py2 = python2 ./fmap_unittest.py +fmap_unittest py3 = python3 ./fmap_unittest.py +cros lint = cros lint ${PRESUBMIT_FILES} [Hook Overrides] cros_license_check: false diff --git a/fmap.py b/fmap.py index a5ec8ef..0ef9f40 100755 --- a/fmap.py +++ b/fmap.py @@ -1,4 +1,5 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- # # Copyright 2010, Google Inc. # All rights reserved. @@ -49,8 +50,10 @@ tuple of decoded area flags. """ +from __future__ import print_function import argparse +import copy import logging import pprint import struct @@ -58,7 +61,7 @@ # constants imported from lib/fmap.h -FMAP_SIGNATURE = '__FMAP__' +FMAP_SIGNATURE = b'__FMAP__' FMAP_VER_MAJOR = 1 FMAP_VER_MINOR_MIN = 0 FMAP_VER_MINOR_MAX = 1 @@ -112,7 +115,15 @@ def _fmap_decode_header(blob, offset): raise struct.error('Incompatible version') # convert null-terminated names - header['name'] = header['name'].strip(chr(0)) + header['name'] = header['name'].strip(b'\x00') + + # In Python 2, binary==string, so we don't need to convert. + if sys.version_info.major >= 3: + # Do the decode after verifying it to avoid decode errors due to corruption. + for name in FMAP_HEADER_NAMES: + if hasattr(header[name], 'decode'): + header[name] = header[name].decode('utf-8') + return (header, struct.calcsize(FMAP_HEADER_FORMAT)) @@ -123,9 +134,16 @@ def _fmap_decode_area(blob, offset): struct.unpack_from(FMAP_AREA_FORMAT, blob, offset)): area[name] = value # convert null-terminated names - area['name'] = area['name'].strip(chr(0)) + area['name'] = area['name'].strip(b'\x00') # add a (readonly) readable FLAGS area['FLAGS'] = _fmap_decode_area_flags(area['flags']) + + # In Python 2, binary==string, so we don't need to convert. + if sys.version_info.major >= 3: + for name in FMAP_AREA_NAMES: + if hasattr(area[name], 'decode'): + area[name] = area[name].decode('utf-8') + return (area, struct.calcsize(FMAP_AREA_FORMAT)) @@ -171,7 +189,7 @@ def _fmap_search_header(blob, fmap_name=None): align *= 2 while align >= FMAP_SEARCH_STRIDE: - for offset in xrange(align, lim + 1, align * 2): + for offset in range(align, lim + 1, align * 2): if not blob.startswith(FMAP_SIGNATURE, offset): continue try: @@ -214,12 +232,24 @@ def fmap_decode(blob, offset=None, fmap_name=None): def _fmap_encode_header(obj): """(internal) Encodes a FMAP header""" + # Convert strings to bytes. + obj = copy.deepcopy(obj) + for name in FMAP_HEADER_NAMES: + if hasattr(obj[name], 'encode'): + obj[name] = obj[name].encode('utf-8') + values = [obj[name] for name in FMAP_HEADER_NAMES] return struct.pack(FMAP_HEADER_FORMAT, *values) def _fmap_encode_area(obj): """(internal) Encodes a FMAP area entry""" + # Convert strings to bytes. + obj = copy.deepcopy(obj) + for name in FMAP_AREA_NAMES: + if hasattr(obj[name], 'encode'): + obj[name] = obj[name].encode('utf-8') + values = [obj[name] for name in FMAP_AREA_NAMES] return struct.pack(FMAP_AREA_FORMAT, *values) diff --git a/fmap_unittest.py b/fmap_unittest.py index 4a5bffa..91b346b 100755 --- a/fmap_unittest.py +++ b/fmap_unittest.py @@ -1,10 +1,14 @@ -#!/usr/bin/python +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- # # Copyright 2017 The Chromium OS Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. + """Unit test for fmap module.""" +from __future__ import print_function + import struct import unittest @@ -54,7 +58,7 @@ class FmapTest(unittest.TestCase): maxDiff = None def setUp(self): - with open('bin/example.bin') as f: + with open('bin/example.bin', 'rb') as f: self.example_blob = f.read() def testDecode(self):