binman: Tidy up setting of entry contents

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>

7 files changed, 62 insertions, 6 deletions

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;
+		};
+	};
+};