[U-Boot,33/53] binman: Add more tests for image header position
diff mbox series

Message ID 20190720182416.183626-34-sjg@chromium.org
State Accepted
Commit eba1f0cc942947722f70029c033b915113cec1ba
Delegated to: Simon Glass
Headers show
Series
  • binman: Support replacing entries in an existing image
Related show

Commit Message

Simon Glass July 20, 2019, 6:23 p.m. UTC
The positioning does not currently work correctly if at the end of an
image with no fixed size. Also if the header is in the middle of an image
it can cause a gap in the image since the header position is normally at
the image end, so entries after it are placed after the end of the image.

Fix these problems and add more tests to cover these cases.

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

 tools/binman/entry.py                         | 15 +++++++++++
 tools/binman/etype/image_header.py            | 16 ++++++++++--
 tools/binman/etype/section.py                 |  9 +++++++
 tools/binman/ftest.py                         | 25 +++++++++++++++++++
 tools/binman/test/135_fdtmap_hdr_middle.dts   | 16 ++++++++++++
 tools/binman/test/136_fdtmap_hdr_startbad.dts | 16 ++++++++++++
 tools/binman/test/137_fdtmap_hdr_endbad.dts   | 16 ++++++++++++
 tools/binman/test/138_fdtmap_hdr_nosize.dts   | 16 ++++++++++++
 8 files changed, 127 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/test/135_fdtmap_hdr_middle.dts
 create mode 100644 tools/binman/test/136_fdtmap_hdr_startbad.dts
 create mode 100644 tools/binman/test/137_fdtmap_hdr_endbad.dts
 create mode 100644 tools/binman/test/138_fdtmap_hdr_nosize.dts

Comments

Simon Glass July 29, 2019, 9:22 p.m. UTC | #1
The positioning does not currently work correctly if at the end of an
image with no fixed size. Also if the header is in the middle of an image
it can cause a gap in the image since the header position is normally at
the image end, so entries after it are placed after the end of the image.

Fix these problems and add more tests to cover these cases.

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

 tools/binman/entry.py                         | 15 +++++++++++
 tools/binman/etype/image_header.py            | 16 ++++++++++--
 tools/binman/etype/section.py                 |  9 +++++++
 tools/binman/ftest.py                         | 25 +++++++++++++++++++
 tools/binman/test/135_fdtmap_hdr_middle.dts   | 16 ++++++++++++
 tools/binman/test/136_fdtmap_hdr_startbad.dts | 16 ++++++++++++
 tools/binman/test/137_fdtmap_hdr_endbad.dts   | 16 ++++++++++++
 tools/binman/test/138_fdtmap_hdr_nosize.dts   | 16 ++++++++++++
 8 files changed, 127 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/test/135_fdtmap_hdr_middle.dts
 create mode 100644 tools/binman/test/136_fdtmap_hdr_startbad.dts
 create mode 100644 tools/binman/test/137_fdtmap_hdr_endbad.dts
 create mode 100644 tools/binman/test/138_fdtmap_hdr_nosize.dts

Applied to u-boot-dm, thanks!

Patch
diff mbox series

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 74e280849c4..8dacc61f5a1 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -745,3 +745,18 @@  features to produce new behaviours.
         ok = self.ProcessContentsUpdate(data)
         self.Detail('WriteData: size=%x, ok=%s' % (len(data), ok))
         return ok
+
+    def GetSiblingOrder(self):
+        """Get the relative order of an entry amoung its siblings
+
+        Returns:
+            'start' if this entry is first among siblings, 'end' if last,
+                otherwise None
+        """
+        entries = list(self.section.GetEntries().values())
+        if entries:
+            if self == entries[0]:
+                return 'start'
+            elif self == entries[-1]:
+                return 'end'
+        return 'middle'
diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py
index 8f9c5aa5d9e..4b69eda1a22 100644
--- a/tools/binman/etype/image_header.py
+++ b/tools/binman/etype/image_header.py
@@ -86,8 +86,20 @@  class Entry_image_header(Entry):
             if self.location not in ['start', 'end']:
                 self.Raise("Invalid location '%s', expected 'start' or 'end'" %
                            self.location)
-            image_size = self.section.GetImageSize() or 0
-            self.offset = (0 if self.location != 'end' else image_size - 8)
+            order = self.GetSiblingOrder()
+            if self.location != order and not self.section.GetSort():
+                self.Raise("Invalid sibling order '%s' for image-header: Must be at '%s' to match location" %
+                           (order, self.location))
+            if self.location != 'end':
+                offset = 0
+            else:
+                image_size = self.section.GetImageSize()
+                if image_size is None:
+                    # We don't know the image, but this must be the last entry,
+                    # so we can assume it goes
+                    offset = offset
+                else:
+                    offset = image_size - IMAGE_HEADER_LEN
         return Entry.Pack(self, offset)
 
     def ProcessContents(self):
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 0fb81419cea..3ce013d5029 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -479,3 +479,12 @@  class Entry_section(Entry):
         if not self.section:
             return self
         return self.section.GetImage()
+
+    def GetSort(self):
+        """Check if the entries in this section will be sorted
+
+        Returns:
+            True if to be sorted, False if entries will be left in the order
+                they appear in the device tree
+        """
+        return self._sort
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index b67e8a15086..bc751893edb 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2886,6 +2886,31 @@  class TestFunctional(unittest.TestCase):
             else:
                 start += dtb._fdt_obj.totalsize()
 
+    def testFdtmapHeaderMiddle(self):
+        """Test an FDT map in the middle of an image when it should be at end"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileRealDtb('135_fdtmap_hdr_middle.dts')
+        self.assertIn("Invalid sibling order 'middle' for image-header: Must be at 'end' to match location",
+                      str(e.exception))
+
+    def testFdtmapHeaderStartBad(self):
+        """Test an FDT map in middle of an image when it should be at start"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileRealDtb('136_fdtmap_hdr_startbad.dts')
+        self.assertIn("Invalid sibling order 'end' for image-header: Must be at 'start' to match location",
+                      str(e.exception))
+
+    def testFdtmapHeaderEndBad(self):
+        """Test an FDT map at the start of an image when it should be at end"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileRealDtb('137_fdtmap_hdr_endbad.dts')
+        self.assertIn("Invalid sibling order 'start' for image-header: Must be at 'end' to match location",
+                      str(e.exception))
+
+    def testFdtmapHeaderNoSize(self):
+        """Test an image header at the end of an image with undefined size"""
+        self._DoReadFileRealDtb('138_fdtmap_hdr_nosize.dts')
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/135_fdtmap_hdr_middle.dts b/tools/binman/test/135_fdtmap_hdr_middle.dts
new file mode 100644
index 00000000000..d6211da8ae3
--- /dev/null
+++ b/tools/binman/test/135_fdtmap_hdr_middle.dts
@@ -0,0 +1,16 @@ 
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		image-header {
+			location = "end";
+		};
+		fdtmap {
+		};
+	};
+};
diff --git a/tools/binman/test/136_fdtmap_hdr_startbad.dts b/tools/binman/test/136_fdtmap_hdr_startbad.dts
new file mode 100644
index 00000000000..ec5f4bc7e3a
--- /dev/null
+++ b/tools/binman/test/136_fdtmap_hdr_startbad.dts
@@ -0,0 +1,16 @@ 
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "start";
+		};
+	};
+};
diff --git a/tools/binman/test/137_fdtmap_hdr_endbad.dts b/tools/binman/test/137_fdtmap_hdr_endbad.dts
new file mode 100644
index 00000000000..ebacd71eb23
--- /dev/null
+++ b/tools/binman/test/137_fdtmap_hdr_endbad.dts
@@ -0,0 +1,16 @@ 
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		image-header {
+			location = "end";
+		};
+		u-boot {
+		};
+		fdtmap {
+		};
+	};
+};
diff --git a/tools/binman/test/138_fdtmap_hdr_nosize.dts b/tools/binman/test/138_fdtmap_hdr_nosize.dts
new file mode 100644
index 00000000000..c362f8fdffb
--- /dev/null
+++ b/tools/binman/test/138_fdtmap_hdr_nosize.dts
@@ -0,0 +1,16 @@ 
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};