At present the contents of an entry are set in subclasses simply by assigning to the data and content_size properties. Add some methods to do this, so that we have more control. In particular, add a method to set the contents without changing its size, so we can validate that case.
Add a test case for trying to change the size when this is not allowed.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: None
tools/binman/entry.py | 28 +++++++++++++++++++++ tools/binman/etype/_testing.py | 8 ++++++ tools/binman/etype/blob.py | 3 +-- tools/binman/etype/u_boot_spl_bss_pad.py | 3 +-- tools/binman/etype/u_boot_with_ucode_ptr.py | 4 +-- tools/binman/ftest.py | 8 ++++++ tools/binman/test/59_change_size.dts | 14 +++++++++++ 7 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 tools/binman/test/59_change_size.dts
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e4d688c91f..303c992e37 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -55,6 +55,7 @@ class Entry(object): self.name = node and (name_prefix + node.name) or 'none' self.pos = None self.size = None + self.data = '' self.contents_size = 0 self.align = None self.align_size = None @@ -138,6 +139,33 @@ class Entry(object): if prefix: self.name = prefix + self.name
+ def SetContents(self, data): + """Set the contents of an entry + + This sets both the data and content_size properties + + Args: + data: Data to set to the contents (string) + """ + self.data = data + self.contents_size = len(self.data) + + def ProcessContentsUpdate(self, data): + """Update the contens of an entry, after the size is fixed + + This checks that the new data is the same size as the old. + + Args: + data: Data to set to the contents (string) + + Raises: + ValueError if the new data size is not the same as the old + """ + if len(data) != self.contents_size: + self.Raise('Cannot update entry size from %d to %d' % + (len(data), self.contents_size)) + self.SetContents(data) + def ObtainContents(self): """Figure out the contents of an entry.
diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index c075c3ff0d..04bdc6c532 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -22,6 +22,8 @@ class Entry__testing(Entry): 'return-invalid-entry') self.return_unknown_contents = fdt_util.GetBool(self._node, 'return-unknown-contents') + self.bad_update_contents = fdt_util.GetBool(self._node, + 'bad-update-contents')
def ObtainContents(self): if self.return_unknown_contents: @@ -34,3 +36,9 @@ class Entry__testing(Entry): if self.return_invalid_entry : return {'invalid-entry': [1, 2]} return {} + + def ProcessContents(self): + if self.bad_update_contents: + # Request to update the conents with something larger, to cause a + # failure. + self.ProcessContentsUpdate('aa') diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 16b1e5f64d..28e6651a93 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -28,8 +28,7 @@ class Entry_blob(Entry): # new Entry method which can read in chunks. Then we could copy # the data in chunks and avoid reading it all at once. For now # this seems like an unnecessary complication. - self.data = fd.read() - self.contents_size = len(self.data) + self.SetContents(fd.read()) return True
def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index 6c397957e3..65f631d3c5 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -22,6 +22,5 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): bss_size = elf.GetSymbolAddress(fname, '__bss_size') if not bss_size: self.Raise('Expected __bss_size symbol in spl/u-boot-spl') - self.data = chr(0) * bss_size - self.contents_size = bss_size + self.SetContents(chr(0) * bss_size) return True diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 41c2ded2fe..86945f3318 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -81,5 +81,5 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): # Write the microcode position and size into the entry pos_and_size = struct.pack('<2L', pos, size) self.target_pos -= self.pos - self.data = (self.data[:self.target_pos] + pos_and_size + - self.data[self.target_pos + 8:]) + self.ProcessContentsUpdate(self.data[:self.target_pos] + pos_and_size + + self.data[self.target_pos + 8:]) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ca9d158eef..af3b4dc3e5 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1006,5 +1006,13 @@ class TestFunctional(unittest.TestCase): "processing of contents: remaining [<_testing.Entry__testing ", str(e.exception))
+ def testBadChangeSize(self): + """Test that trying to change the size of an entry fails""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('59_change_size.dts', True) + self.assertIn("Node '/binman/_testing': Cannot update entry size from " + '2 to 1', str(e.exception)) + + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/59_change_size.dts b/tools/binman/test/59_change_size.dts new file mode 100644 index 0000000000..1a69026a64 --- /dev/null +++ b/tools/binman/test/59_change_size.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + }; + }; +};