[PATCH 1/6] binman: Require image filenames to have certain extensions

[Simon Glass]

It is helpful to be able to distinguish output files from other files when
building U-Boot. This is useful for buildman, for example, which can
preserve output files when requested.

Most images use extensions like .bin or .rom but some use other ones.

Introduce a check and produce an error if an image filename does not have
an allowed extension.

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

 tools/binman/binman.rst                  |  5 +++++
 tools/binman/etype/section.py            |  3 +--
 tools/binman/ftest.py                    | 12 ++++++++++--
 tools/binman/image.py                    |  9 +++++++++
 tools/binman/test/022_image_name.dts     |  4 ++--
 tools/binman/test/311_bad_image_name.dts | 17 +++++++++++++++++
 6 files changed, 44 insertions(+), 6 deletions(-)
 create mode 100644 tools/binman/test/311_bad_image_name.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index aeea33fddb95..3c24a8313c47 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -678,6 +678,11 @@ filename:
     put into the entry. If binman knows about the entry type (like
     u-boot-bin), then there is no need to specify this.
 
+    Note that image filenames must have one of a limited set of extensions:
+    `.bin`, `.rom`, `.itb` or `.img`. Others will generate an error. This is
+    so that it is clear which output files are images. It allows buildman to
+    preserve them when its `-k` option is used, for example.
+
 type:
     Sets the type of an entry. This defaults to the entry name, but it is
     possible to use any name, and then add (for example) 'type = "u-boot"'
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index fb49e85a7634..8074f181ea58 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -10,6 +10,7 @@ images to be created.
 
 from collections import OrderedDict
 import concurrent.futures
+import os
 import re
 import sys
 
@@ -20,7 +21,6 @@ from u_boot_pylib import tools
 from u_boot_pylib import tout
 from u_boot_pylib.tools import to_hex_size
 
-
 class Entry_section(Entry):
     """Entry that contains other entries
 
@@ -203,7 +203,6 @@ class Entry_section(Entry):
         self.align_default = fdt_util.GetInt(self._node, 'align-default', 0)
         self._filename = fdt_util.GetString(self._node, 'filename',
                                             self._filename)
-
         self.ReadEntries()
 
     def ReadEntries(self):
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 1293e9dbf423..5b54239827d4 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1107,11 +1107,11 @@ class TestFunctional(unittest.TestCase):
         retcode = self._DoTestFile('022_image_name.dts')
         self.assertEqual(0, retcode)
         image = control.images['image1']
-        fname = tools.get_output_filename('test-name')
+        fname = tools.get_output_filename('test-name.bin')
         self.assertTrue(os.path.exists(fname))
 
         image = control.images['image2']
-        fname = tools.get_output_filename('test-name.xx')
+        fname = tools.get_output_filename('test-name.img')
         self.assertTrue(os.path.exists(fname))
 
     def testBlobFilename(self):
@@ -7216,5 +7216,13 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
         self.assertRegex(err,
                          "Image 'image'.*missing bintools.*: bootgen")
 
+    def testBadImageName(self):
+        """Test that image files can be named"""
+        with self.assertRaises(ValueError) as e:
+            self._DoTestFile('311_bad_image_name.dts')
+        self.assertIn(
+            "Image filename 'test-name.what' must use a permitted extension: .bin, .rom, .itb, .img",
+            str(e.exception))
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index e77b5d0d97cd..71dc28854412 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -21,6 +21,9 @@ from dtoc import fdt_util
 from u_boot_pylib import tools
 from u_boot_pylib import tout
 
+# Extensions allowed for images (keep in sync with buildman/builderthread.py)
+ALLOWED_EXTS = ['.bin', '.rom', '.itb', '.img']
+
 class Image(section.Entry_section):
     """A Image, representing an output from binman
 
@@ -97,6 +100,12 @@ class Image(section.Entry_section):
         self.allow_repack = fdt_util.GetBool(self._node, 'allow-repack')
         self._symlink = fdt_util.GetString(self._node, 'symlink')
 
+        if self.section is None:
+            ext = os.path.splitext(self._filename)[1]
+            if ext not in ALLOWED_EXTS:
+                self.Raise(
+                    f"Image filename '{self._filename}' must use a permitted extension: {', '.join(ALLOWED_EXTS)}")
+
     @classmethod
     def FromFile(cls, fname):
         """Convert an image file into an Image for use in binman
diff --git a/tools/binman/test/022_image_name.dts b/tools/binman/test/022_image_name.dts
index 94fc069c1764..2da68ab06921 100644
--- a/tools/binman/test/022_image_name.dts
+++ b/tools/binman/test/022_image_name.dts
@@ -7,13 +7,13 @@
 	binman {
 		multiple-images;
 		image1 {
-			filename = "test-name";
+			filename = "test-name.bin";
 			u-boot {
 			};
 		};
 
 		image2 {
-			filename = "test-name.xx";
+			filename = "test-name.img";
 			u-boot {
 			};
 		};
diff --git a/tools/binman/test/311_bad_image_name.dts b/tools/binman/test/311_bad_image_name.dts
new file mode 100644
index 000000000000..30192957c447
--- /dev/null
+++ b/tools/binman/test/311_bad_image_name.dts
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		multiple-images;
+		image1 {
+			filename = "test-name.what";
+			u-boot {
+			};
+		};
+	};
+};
-- 
2.42.0.rc1.204.g551eb34607-goog