binman: Support shrinking a entry after packing

Sometimes an entry may shrink after it has already been packed. In that
case we must repack the items. Of course it is always possible to just
leave the entry at its original size and waste space at the end. This is
what binman does by default, since there is the possibility of the entry
changing size every time binman calculates its contents, thus causing a
loop.

Signed-off-by: Simon Glass <sjg@chromium.org>

diff --git a/tools/binman/control.py b/tools/binman/control.py
index c3f358d45c40a2e5afaa0047aeb8bfbce86f2e0a..22e3e306e54326a6216084fd11bf854ba760e48c 100644 (file)
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -333,7 +333,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
             break
         image.ResetForPack()
     if not sizes_ok:
-        image.Raise('Entries expanded after packing (tried %s passes)' %
+        image.Raise('Entries changed size after packing (tried %s passes)' %
                     passes)
 
     image.WriteSymbols()

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 8dacc61f5a1b467271bf6b041637f5172f575986..90192c11b7788db1ee97d912d24e23eb66606b74 100644 (file)
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -285,16 +285,26 @@ class Entry(object):
         """
         size_ok = True
         new_size = len(data)
-        if state.AllowEntryExpansion():
+        if state.AllowEntryExpansion() and new_size > self.contents_size:
+            # self.data will indicate the new size needed
+            size_ok = False
+        elif state.AllowEntryContraction() and new_size < self.contents_size:
+            size_ok = False
+
+        # If not allowed to change, try to deal with it or give up
+        if size_ok:
             if new_size > self.contents_size:
-                tout.Debug("Entry '%s' size change from %s to %s" % (
-                    self._node.path, ToHex(self.contents_size),
-                    ToHex(new_size)))
-                # self.data will indicate the new size needed
-                size_ok = False
-        elif new_size != self.contents_size:
-            self.Raise('Cannot update entry size from %d to %d' %
-                       (self.contents_size, new_size))
+                self.Raise('Cannot update entry size from %d to %d' %
+                        (self.contents_size, new_size))
+
+            # Don't let the data shrink. Pad it if necessary
+            if size_ok and new_size < self.contents_size:
+                data += tools.GetBytes(0, self.contents_size - new_size)
+
+        if not size_ok:
+            tout.Debug("Entry '%s' size change from %s to %s" % (
+                self._node.path, ToHex(self.contents_size),
+                ToHex(new_size)))
         self.SetContents(data)
         return size_ok
 

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index f8568d7cda1105c283547a72b16618e8cfe32def..11155ced70974319c9bcc2614a6e2ef6b9d0d927 100644 (file)
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2143,7 +2143,7 @@ class TestFunctional(unittest.TestCase):
         """Test expanding an entry after it is packed, twice"""
         with self.assertRaises(ValueError) as e:
             self._DoReadFile('122_entry_expand_twice.dts')
-        self.assertIn("Image '/binman': Entries expanded after packing",
+        self.assertIn("Image '/binman': Entries changed size after packing",
                       str(e.exception))
 
     def testEntryExpandSection(self):
@@ -2952,6 +2952,29 @@ class TestFunctional(unittest.TestCase):
         self.assertIn('Entry data size does not match, but allow-repack is not present for this image',
                       str(e.exception))
 
+    def testEntryShrink(self):
+        """Test contracting an entry after it is packed"""
+        try:
+            state.SetAllowEntryContraction(True)
+            data = self._DoReadFileDtb('140_entry_shrink.dts',
+                                       update_dtb=True)[0]
+        finally:
+            state.SetAllowEntryContraction(False)
+        self.assertEqual(b'a', data[:1])
+        self.assertEqual(U_BOOT_DATA, data[1:1 + len(U_BOOT_DATA)])
+        self.assertEqual(b'a', data[-1:])
+
+    def testEntryShrinkFail(self):
+        """Test not being allowed to contract an entry after it is packed"""
+        data = self._DoReadFileDtb('140_entry_shrink.dts', update_dtb=True)[0]
+
+        # In this case there is a spare byte at the end of the data. The size of
+        # the contents is only 1 byte but we still have the size before it
+        # shrunk.
+        self.assertEqual(b'a\0', data[:2])
+        self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)])
+        self.assertEqual(b'a\0', data[-2:])
+
 
 if __name__ == "__main__":
     unittest.main()

diff --git a/tools/binman/state.py b/tools/binman/state.py
index 65536151b44adf83891b585e0e6bda49600330ac..f22cc82d870cc77ac34164257eeb005927cf3b5c 100644 (file)
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -42,6 +42,14 @@ main_dtb = None
 # Entry.ProcessContentsUpdate()
 allow_entry_expansion = True
 
+# Don't allow entries to contract after they have been packed. Instead just
+# leave some wasted space. If allowed, this is detected and forces a re-pack,
+# but may result in entries that oscillate in size, thus causing a pack error.
+# An example is a compressed device tree where the original offset values
+# result in a larger compressed size than the new ones, but then after updating
+# to the new ones, the compressed size increases, etc.
+allow_entry_contraction = False
+
 def GetFdtForEtype(etype):
     """Get the Fdt object for a particular device-tree entry
 
@@ -346,3 +354,22 @@ def AllowEntryExpansion():
             raised
     """
     return allow_entry_expansion
+
+def SetAllowEntryContraction(allow):
+    """Set whether post-pack contraction of entries is allowed
+
+    Args:
+       allow: True to allow contraction, False to raise an exception
+    """
+    global allow_entry_contraction
+
+    allow_entry_contraction = allow
+
+def AllowEntryContraction():
+    """Check whether post-pack contraction of entries is allowed
+
+    Returns:
+        True if contraction should be allowed, False if an exception should be
+            raised
+    """
+    return allow_entry_contraction

diff --git a/tools/binman/test/140_entry_shrink.dts b/tools/binman/test/140_entry_shrink.dts
new file mode 100644 (file)
index 0000000..b750d63
--- /dev/null
+++ b/tools/binman/test/140_entry_shrink.dts
@@ -0,0 +1,20 @@
+/dts-v1/;
+
+/ {
+       #address-cells = <1>;
+       #size-cells = <1>;
+
+       binman {
+               _testing {
+                       bad-shrink-contents;
+               };
+
+               u-boot {
+               };
+
+               _testing2 {
+                       type = "_testing";
+                       bad-shrink-contents;
+               };
+       };
+};