Skip to content

Commit d29be8b

Browse files
UncleGrumpyUncleGrumpy
committed
Fix bugs in esp32 mkimage.erl script
* Fix an FD leak, by properly closing the image file after writing. * Fix a misleading error message when a binary overflows its partition. * Fix an unaligned end of data problem that can prevent flashing with a web-flasher tool. * Catch errors in file:write/2 operations, rather than blindly continue when an error is returned. Closes #2084 Closes #2094 Signed-off-by: Winford <winford@object.stream>
1 parent 8aa89af commit d29be8b

4 files changed

Lines changed: 84 additions & 47 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
- Stop using deprecated `term_from_int32` on RP2 platform
1515
- Stop using deprecated `term_from_int32` on ESP32 platform
1616
- Fixed bug in ESP32 mkimage.sh leading to non-fatal "unary operator expected" error
17+
- Fixed `fd` leak in ESP32 mkimage.erl
18+
- Fixed mkimage.erl unaligned end of data problems that can prevent
19+
flashing with a web-flasher tool
20+
- Fixed misleading error message when a binary overflows its partition
21+
- Fixed mkimage.erl to stop with and error when file:write/2 fails
1722

1823
## [0.7.0-alpha.1] - 2026-04-06
1924

‎src/platforms/esp32/tools/mkimage.config.in‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
% SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
1919
%
2020

21+
%% It is important that segments are always kept in order by offset
22+
%% so mkimage.erl can pad partitons to the correct size.
23+
2124
#{
2225
segments => [
2326
#{

‎src/platforms/esp32/tools/mkimage.erl‎

Lines changed: 73 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -116,57 +116,83 @@ mkimage(OutputFile, Segments) ->
116116

117117
case file:open(OutputFile, [write, binary]) of
118118
{ok, Fout} ->
119-
lists:foldl(
120-
fun(Segment, PrevOffset) ->
121-
SegmentOffset = from_hex(maps:get(offset, Segment)),
122-
case PrevOffset of
123-
undefined ->
124-
no_padding;
125-
_ ->
126-
case SegmentOffset > PrevOffset of
127-
true ->
128-
Padding = [
129-
16#FF
130-
|| _ <- lists:seq(1, SegmentOffset - PrevOffset)
131-
],
132-
io:format("Padding ~p bytes~n", [SegmentOffset - PrevOffset]),
133-
file:write(Fout, Padding);
134-
false ->
135-
throw(
136-
io_lib:format(
137-
"Error: insufficient space for segment ~p. Over by: ~p bytes~n",
138-
[
139-
maps:get(name, Segment), PrevOffset - SegmentOffset
140-
]
141-
)
142-
)
143-
end
144-
end,
145-
SegmentPaths = maps:get(path, Segment),
146-
case try_read(SegmentPaths) of
147-
{ok, Data} ->
148-
file:write(Fout, Data),
149-
io:format("Wrote ~s (~p bytes) at offset ~s (~p)~n", [
150-
maps:get(name, Segment),
151-
byte_size(Data),
152-
maps:get(offset, Segment),
153-
SegmentOffset
154-
]),
155-
SegmentOffset + byte_size(Data);
156-
{error, Reason} ->
157-
Fmt =
158-
"Failed to read file ~p Reason: ~p."
159-
" Note that a full build is required before running this command.",
160-
throw(io_lib:format(Fmt, [SegmentPaths, Reason]))
161-
end
162-
end,
163-
undefined,
164-
Segments
165-
);
119+
try
120+
write_segments(Fout, Segments)
121+
after
122+
file:close(Fout)
123+
end;
166124
{error, Reason} ->
167125
throw(io_lib:format("Failed to open ~s for writing. Reason: ~p", [OutputFile, Reason]))
168126
end.
169127

128+
%% @private
129+
write_segments(_Fout, []) ->
130+
ok;
131+
write_segments(Fout, [Segment | Segments]) ->
132+
SegmentOffset = from_hex(maps:get(offset, Segment)),
133+
SegmentPaths = maps:get(path, Segment),
134+
SegmentEnd =
135+
case try_read(SegmentPaths) of
136+
{ok, Data} ->
137+
case file:write(Fout, Data) of
138+
ok ->
139+
ok;
140+
{error, WriteError} ->
141+
Fmt = "Failed to write segment data from ~p to image. Reason: ~p.",
142+
throw(io_lib:format(Fmt, [SegmentPaths, WriteError]))
143+
end,
144+
io:format("Wrote ~s (~p bytes) at offset ~s (~p)~n", [
145+
maps:get(name, Segment),
146+
byte_size(Data),
147+
maps:get(offset, Segment),
148+
SegmentOffset
149+
]),
150+
SegmentOffset + byte_size(Data);
151+
{error, Reason} ->
152+
Fmt =
153+
"Failed to read file ~p Reason: ~p."
154+
" Note that a full build is required before running this command.",
155+
throw(io_lib:format(Fmt, [SegmentPaths, Reason]))
156+
end,
157+
if
158+
Segments == [] ->
159+
%% Pad to end on 32-byte alignment, since we don't have a next offset to pad to.
160+
SectorPad = (32 - (SegmentEnd rem 32)) rem 32,
161+
pad_to_size(Fout, SectorPad);
162+
true ->
163+
[PeekNext | _] = Segments,
164+
NextOffset = from_hex(maps:get(offset, PeekNext)),
165+
case SegmentEnd > NextOffset of
166+
true ->
167+
throw(
168+
io_lib:format(
169+
"Error: insufficient space for segment ~p. Overflows partition by: ~p bytes~n",
170+
[maps:get(name, Segment), SegmentEnd - NextOffset]
171+
)
172+
);
173+
false ->
174+
PadSize = NextOffset - SegmentEnd,
175+
case PadSize of
176+
0 ->
177+
ok;
178+
PadSize ->
179+
ok = pad_to_size(Fout, PadSize)
180+
end
181+
end
182+
end,
183+
write_segments(Fout, Segments).
184+
185+
%% @private
186+
pad_to_size(Fout, PadSize) ->
187+
Padding = [16#FF || _ <- lists:seq(1, PadSize)],
188+
case file:write(Fout, Padding) of
189+
ok ->
190+
io:format("Padded ~p bytes~n", [PadSize]);
191+
{error, Reason} ->
192+
Fmt = "Failed to add ~p bytes padding. Reason: ~p.",
193+
throw(io_lib:format(Fmt, [PadSize, Reason]))
194+
end.
195+
170196
%% @private
171197
try_read([]) ->
172198
{error, not_found};

‎src/platforms/esp32/tools/mkimage_nvs.config.in‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
% SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
2020
%
2121

22+
%% It is important that segments are always kept in order by offset
23+
%% so mkimage.erl can pad partitons to the correct size.
24+
2225
#{
2326
segments => [
2427
#{

0 commit comments

Comments
 (0)