Fix bugs in esp32 mkimage.erl script

* Fixes an FD leak, the image file was never closed properly after writing.
* Fixes a misleading error message when a binary overflows its partition.
* Avoids possibly reading possibly corrupt old data beyond the end of the last
segment written by padding the remainder of the last sector (flash sectors are
4096 blocks in length) with 0xff. Having this buffer marked as erased between
the end of the partition data and any bits left on the device from previously
flashed projects should prevent accidentally ace ping this as part of the
partition data.

Closes #2084

Signed-off-by: Winford <winford@object.stream>

Author: UncleGrumpy

src/platforms/esp32/tools/mkimage.erl

@@ -135,62 +135,76 @@ mkimage(RootDir, BootFile, OutputFile, Segments) ->
     io:format("=============================================~n"),
     case file:open(OutputFile, [write, binary]) of
         {ok, Fout} ->
-            lists:foldl(
-                fun(Segment, PrevOffset) ->
-                    SegmentOffset = from_hex(maps:get(offset, Segment)),
-                    case PrevOffset of
-                        undefined ->
-                            no_padding;
-                        _ ->
-                            case SegmentOffset > PrevOffset of
-                                true ->
-                                    Padding = [
-                                        16#FF
-                                     || _ <- lists:seq(1, SegmentOffset - PrevOffset)
-                                    ],
-                                    io:format("Padding ~p bytes~n", [SegmentOffset - PrevOffset]),
-                                    file:write(Fout, Padding);
-                                false ->
-                                    throw(
-                                        io_lib:format(
-                                            "Error: insufficient space for segment ~p.  Over by: ~p bytes~n",
-                                            [
-                                                maps:get(name, Segment), PrevOffset - SegmentOffset
-                                            ]
-                                        )
-                                    )
-                            end
-                    end,
-                    SegmentPaths = [
-                        replace(
-                            "BOOT_FILE", BootFile, replace("ROOT_DIR", RootDir, SegmentPath)
-                        )
-                     || SegmentPath <- maps:get(path, Segment)
-                    ],
-                    case try_read(SegmentPaths) of
-                        {ok, Data} ->
-                            file:write(Fout, Data),
-                            io:format("Wrote ~s (~p bytes) at offset ~s (~p)~n", [
-                                maps:get(name, Segment),
-                                byte_size(Data),
-                                maps:get(offset, Segment),
-                                SegmentOffset
-                            ]),
-                            SegmentOffset + byte_size(Data);
-                        {error, Reason} ->
-                            Fmt =
-                                "Failed to read file ~p  Reason: ~p."
-                                "  Note that a full build is required before running this command.",
-                            throw(io_lib:format(Fmt, [SegmentPaths, Reason]))
-                    end
-                end,
-                undefined,
-                Segments
-            );
+            write_segments(RootDir, BootFile, Fout, Segments);
         {error, Reason} ->
             throw(io_lib:format("Failed to open ~s for writing.  Reason: ~p", [OutputFile, Reason]))
     end.
 
+%% @private
+write_segments(_RootDir, _BootFile, Fout, []) ->
+    file:close(Fout);
+write_segments(RootDir, BootFile, Fout, [Segment | Segments]) ->
+    SegmentOffset = from_hex(maps:get(offset, Segment)),
+    SegmentPaths = [
+        replace(
+            "BOOT_FILE", BootFile, replace("ROOT_DIR", RootDir, SegmentPath)
+        )
+     || SegmentPath <- maps:get(path, Segment)
+    ],
+    SegmentEnd =
+        case try_read(SegmentPaths) of
+            {ok, Data} ->
+                file:write(Fout, Data),
+                io:format("Wrote ~s (~p bytes) at offset ~s (~p)~n", [
+                    maps:get(name, Segment),
+                    byte_size(Data),
+                    maps:get(offset, Segment),
+                    SegmentOffset
+                ]),
+                SegmentOffset + byte_size(Data);
+            {error, Reason} ->
+                Fmt =
+                    "Failed to read file ~p  Reason: ~p."
+                    "  Note that a full build is required before running this command.",
+                throw(io_lib:format(Fmt, [SegmentPaths, Reason]))
+        end,
+    if
+        Segments == [] ->
+            %% Pad the rest of the sector since we don't have a next offset to pad too
+            SectorPad = 4096 - (SegmentEnd rem 4096),
+            case SectorPad of
+                0 -> ok;
+                _ -> ok = pad_to_size(Fout, SectorPad)
+            end;
+        true ->
+            [PeekNext | _] = Segments,
+            NextOffset = from_hex(maps:get(offset, PeekNext)),
+            case SegmentEnd > NextOffset of
+                true ->
+                    throw(
+                        io_lib:format(
+                            "Error: insufficient space for segment ~p.  Overflows partition by: ~p bytes~n",
+                            [maps:get(name, Segment), SegmentEnd - NextOffset]
+                        )
+                    );
+                false ->
+                    PadSize = NextOffset - SegmentEnd,
+                    case PadSize of
+                        0 ->
+                            ok;
+                        PadSize ->
+                            ok = pad_to_size(Fout, PadSize)
+                    end
+            end
+    end,
+    write_segments(RootDir, BootFile, Fout, Segments).
+
+%% @private
+pad_to_size(Fout, Size) ->
+    Padding = [16#FF || _ <- lists:seq(1, Size)],
+    io:format("Padding ~p bytes~n", [Size]),
+    file:write(Fout, Padding).
+
 %% @private
 try_read([]) ->
     {error, not_found};