Use subprocess.run in elf2bin (#8799)

Properly handle Popen object, it takes care of everything we need to do like .communicate() and handle timeouts. Introduce a small timeout to run so we also know when that happens
Clean-up with pythonic path join and formatting through f"" strings

Author: mcspr

tools/elf2bin.py

@@ -20,6 +20,7 @@
 
 from __future__ import print_function
 import argparse
+import io
 import re
 import os
 import subprocess
@@ -33,37 +34,75 @@
 crcsize_offset = 4096 + 16
 crcval_offset =  4096 + 16 + 4
 
+
+class Elf2BinException(Exception):
+    pass
+
+
 def get_elf_entry(elf, path):
-    p = subprocess.Popen([path + "/xtensa-lx106-elf-readelf", '-h', elf], stdout=subprocess.PIPE, universal_newlines=True )
-    lines = p.stdout.readlines()
-    for line in lines:
+    result = subprocess.run(
+        [os.path.join(path, "xtensa-lx106-elf-readelf"), "-h", elf],
+        check=True,
+        timeout=15,
+        stdout=subprocess.PIPE,
+        universal_newlines=True,
+    )
+
+    lines = io.StringIO(result.stdout)
+    for line in lines.readlines():
         if 'Entry point address' in line:
             words = re.split('\s+', line)
             entry_point = words[-2]
             return int(entry_point, 16)
-    raise Exception('Unable to find entry point in file "' + elf + '"')
+
+    raise Elf2BinException(f'Unable to find entry point in file "{elf}"')
+
 
 def get_segment_size_addr(elf, segment, path):
-    p = subprocess.Popen([path + '/xtensa-lx106-elf-objdump', '-h', '-j', segment,  elf], stdout=subprocess.PIPE, universal_newlines=True )
-    lines = p.stdout.readlines()
-    for line in lines:
+    result = subprocess.run(
+        [os.path.join(path, "xtensa-lx106-elf-objdump"), "-h", "-j", segment, elf],
+        check=True,
+        timeout=15,
+        stdout=subprocess.PIPE,
+        universal_newlines=True,
+    )
+
+    lines = io.StringIO(result.stdout)
+    for line in lines.readlines():
         if segment in line:
             words = re.split('\s+', line)
             size = int(words[3], 16)
             addr = int(words[4], 16)
             return [ size, addr ]
-    raise Exception('Unable to find size and start point in file "' + elf + '" for "' + segment + '"')
+
+    raise Elf2BinException(
+        f'Unable to find size and start point in file "{elf}" for "{segment}"'
+    )
+
 
 def read_segment(elf, segment, path):
     fd, tmpfile = tempfile.mkstemp()
     os.close(fd)
-    subprocess.check_call([path + "/xtensa-lx106-elf-objcopy", '-O', 'binary', '--only-section=' + segment, elf, tmpfile], stdout=subprocess.PIPE)
-    with open(tmpfile, "rb") as f:
-        raw = f.read()
-    os.remove(tmpfile)
+    try:
+        subprocess.check_call(
+            [
+                os.path.join(path, "xtensa-lx106-elf-objcopy"),
+                "-O",
+                "binary",
+                f"--only-section={segment}",
+                elf,
+                tmpfile,
+            ],
+            stdout=subprocess.PIPE,
+        )
+        with open(tmpfile, "rb") as f:
+            raw = f.read()
+    finally:
+        os.remove(tmpfile)
 
     return raw
 
+
 def write_bin(out, args, elf, segments, to_addr):
     entry = int(get_elf_entry( elf, args.path ))
     header = [ 0xe9, len(segments), fmodeb[args.flash_mode], ffreqb[args.flash_freq] + 16 * fsizeb[args.flash_size],
@@ -85,7 +124,7 @@ def write_bin(out, args, elf, segments, to_addr):
         try:
             for data in raw:
                 checksum = checksum ^ ord(data)
-        except Exception:
+        except:
             for data in raw:
                 checksum = checksum ^ data
     total_size += 1
@@ -95,11 +134,14 @@ def write_bin(out, args, elf, segments, to_addr):
     out.write(bytearray([checksum]))
     if to_addr != 0:
         if total_size + 8 > to_addr:
-            raise Exception('Bin image of ' + elf + ' is too big, actual size ' + str(total_size  + 8) + ', target size ' + str(to_addr) + '.')
+            raise Elf2BinException(
+                f'Bin image of "{elf}" is too big! Actual size {str(total_size  + 8)}, target size {str(to_addr)}'
+            )
         while total_size < to_addr:
             out.write(bytearray([0xaa]))
             total_size += 1
 
+
 def crc8266(ldata):
     "Return the CRC of ldata using same algorithm as eboot"
     crc = 0xffffffff
@@ -119,6 +161,7 @@ def crc8266(ldata):
                 crc = int(crc ^ 0x04c11db7)
     return crc
 
+
 def store_word(raw, offset, val):
     "Place a 4-byte word in 8266-dependent order in the raw image"
     raw[offset] = val & 255
@@ -127,6 +170,7 @@ def store_word(raw, offset, val):
     raw[offset + 3] = (val >> 24) & 255
     return raw
 
+
 def add_crc(out):
     with open(out, "rb") as binfile:
         raw = bytearray(binfile.read())
@@ -141,15 +185,16 @@ def add_crc(out):
     with open(out, "wb") as binfile:
         binfile.write(raw)
 
+
 def gzip_bin(mode, out):
     import gzip
 
     firmware_path = out
-    gzip_path = firmware_path + '.gz'
-    orig_path = firmware_path + '.orig'
+    gzip_path = f"{firmware_path}.gz"
+    orig_path = f"{firmware_path}.orig"
     if os.path.exists(gzip_path):
         os.remove(gzip_path)
-    print('GZipping firmware ' + firmware_path)
+    print(f'GZipping firmware {firmware_path}')
     with open(firmware_path, 'rb') as firmware_file, \
             gzip.open(gzip_path, 'wb') as dest:
         data = firmware_file.read()
@@ -162,10 +207,11 @@ def gzip_bin(mode, out):
     if mode == "PIO":
         if os.path.exists(orig_path):
             os.remove(orig_path)
-        print('Moving original firmware to ' + orig_path)
+        print(f'Moving original firmware to {orig_path}')
         os.rename(firmware_path, orig_path)
         os.rename(gzip_path, firmware_path)
 
+
 def main():
     parser = argparse.ArgumentParser(description='Create a BIN file from eboot.elf and Arduino sketch.elf for upload by esptool.py')
     parser.add_argument('-e', '--eboot', action='store', required=True, help='Path to the Arduino eboot.elf bootloader')
@@ -179,8 +225,7 @@ def main():
 
     args = parser.parse_args()
 
-    print('Creating BIN file "{out}" using "{eboot}" and "{app}"'.format(
-        out=args.out, eboot=args.eboot, app=args.app))
+    print(f'Creating BIN file "{args.out}" using "{args.eboot}" and "{args.app}"')
 
     with open(args.out, "wb") as out:
         def wrapper(**kwargs):