Skip to content

Commit 11704f3

Browse files
UncleGrumpyUncleGrumpy
committed
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. * Fixes and unaligned end of data problems that can prevent flashing with a web-flasher tool. * Now catches 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 de2b14d commit 11704f3

2 files changed

Lines changed: 124 additions & 45 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
### Fixed
1313
- Fixed bug in ESP32 mkimage.sh leading to non-fatal "unary operator expected" error
14+
- Fixed `fd` leak in ESP32 mkimage.erl
1415

1516
## [0.7.0-alpha.1] - 2026-04-06
1617

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

Lines changed: 123 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -116,57 +116,135 @@ 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-
]
119+
try
120+
lists:foldl(
121+
fun(Segment, PrevOffset) ->
122+
SegmentOffset = from_hex(maps:get(offset, Segment)),
123+
case PrevOffset of
124+
undefined ->
125+
no_padding;
126+
_ ->
127+
case SegmentOffset > PrevOffset of
128+
true ->
129+
Padding = [
130+
16#FF
131+
|| _ <- lists:seq(1, SegmentOffset - PrevOffset)
132+
],
133+
io:format("Padding ~p bytes~n", [SegmentOffset - PrevOffset]),
134+
file:write(Fout, Padding);
135+
false ->
136+
throw(
137+
io_lib:format(
138+
"Error: insufficient space for segment ~p. Over by: ~p bytes~n",
139+
[
140+
maps:get(name, Segment),
141+
PrevOffset - SegmentOffset
142+
]
143+
)
141144
)
142-
)
143-
end
145+
end
146+
end,
147+
SegmentPaths = maps:get(path, Segment),
148+
case try_read(SegmentPaths) of
149+
{ok, Data} ->
150+
file:write(Fout, Data),
151+
io:format("Wrote ~s (~p bytes) at offset ~s (~p)~n", [
152+
maps:get(name, Segment),
153+
byte_size(Data),
154+
maps:get(offset, Segment),
155+
SegmentOffset
156+
]),
157+
SegmentOffset + byte_size(Data);
158+
{error, Reason} ->
159+
Fmt =
160+
"Failed to read file ~p Reason: ~p."
161+
" Note that a full build is required before running this command.",
162+
throw(io_lib:format(Fmt, [SegmentPaths, Reason]))
163+
end
144164
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-
);
165+
undefined,
166+
Segments
167+
)
168+
after
169+
file:close(Fout)
170+
end;
166171
{error, Reason} ->
167172
throw(io_lib:format("Failed to open ~s for writing. Reason: ~p", [OutputFile, Reason]))
168173
end.
169174

175+
%% @private
176+
write_segments(_RootDir, _BootFile, Fout, []) ->
177+
ok;
178+
write_segments(RootDir, BootFile, Fout, [Segment | Segments]) ->
179+
SegmentOffset = from_hex(maps:get(offset, Segment)),
180+
SegmentPaths = [
181+
replace(
182+
"BOOT_FILE", BootFile, replace("ROOT_DIR", RootDir, SegmentPath)
183+
)
184+
|| SegmentPath <- maps:get(path, Segment)
185+
],
186+
SegmentEnd =
187+
case try_read(SegmentPaths) of
188+
{ok, Data} ->
189+
case file:write(Fout, Data) of
190+
ok ->
191+
ok;
192+
{error, WriteError} ->
193+
Fmt = "Failed to write segment data from ~p to image. Reason: ~p.",
194+
throw(io_lib:format(Fmt, [SegmentPaths, WriteError]))
195+
end,
196+
io:format("Wrote ~s (~p bytes) at offset ~s (~p)~n", [
197+
maps:get(name, Segment),
198+
byte_size(Data),
199+
maps:get(offset, Segment),
200+
SegmentOffset
201+
]),
202+
SegmentOffset + byte_size(Data);
203+
{error, Reason} ->
204+
Fmt =
205+
"Failed to read file ~p Reason: ~p."
206+
" Note that a full build is required before running this command.",
207+
throw(io_lib:format(Fmt, [SegmentPaths, Reason]))
208+
end,
209+
if
210+
Segments == [] ->
211+
%% Pad to end on 32-byte alignment, since we don't have a next offset to pad to.
212+
SectorPad = (32 - (SegmentEnd rem 32)) rem 32,
213+
pad_to_size(Fout, SectorPad);
214+
true ->
215+
[PeekNext | _] = Segments,
216+
NextOffset = from_hex(maps:get(offset, PeekNext)),
217+
case SegmentEnd > NextOffset of
218+
true ->
219+
throw(
220+
io_lib:format(
221+
"Error: insufficient space for segment ~p. Overflows partition by: ~p bytes~n",
222+
[maps:get(name, Segment), SegmentEnd - NextOffset]
223+
)
224+
);
225+
false ->
226+
PadSize = NextOffset - SegmentEnd,
227+
case PadSize of
228+
0 ->
229+
ok;
230+
PadSize ->
231+
ok = pad_to_size(Fout, PadSize)
232+
end
233+
end
234+
end,
235+
write_segments(RootDir, BootFile, Fout, Segments).
236+
237+
%% @private
238+
pad_to_size(Fout, PadSize) ->
239+
Padding = [16#FF || _ <- lists:seq(1, PadSize)],
240+
case file:write(Fout, Padding) of
241+
ok ->
242+
io:format("Padded ~p bytes~n", [PadSize]);
243+
{error, Reason} ->
244+
Fmt = "Failed to add ~p bytes padding. Reason: ~p.",
245+
throw(io_lib:format(Fmt, [PadSize, Reason]))
246+
end.
247+
170248
%% @private
171249
try_read([]) ->
172250
{error, not_found};

0 commit comments

Comments
 (0)