Skip to content

Commit 0b8b682

Browse files
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 a7c2109 commit 0b8b682

1 file changed

Lines changed: 77 additions & 51 deletions

File tree

src/platforms/esp32/tools/mkimage.erl

Lines changed: 77 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -135,60 +135,86 @@ mkimage(RootDir, BootFile, OutputFile, Segments) ->
135135
io:format("=============================================~n"),
136136
case file:open(OutputFile, [write, binary]) of
137137
{ok, Fout} ->
138-
lists:foldl(
139-
fun(Segment, PrevOffset) ->
140-
SegmentOffset = from_hex(maps:get(offset, Segment)),
141-
case PrevOffset of
142-
undefined ->
143-
no_padding;
144-
_ ->
145-
case SegmentOffset > PrevOffset of
146-
true ->
147-
Padding = [
148-
16#FF
149-
|| _ <- lists:seq(1, SegmentOffset - PrevOffset)
150-
],
151-
io:format("Padding ~p bytes~n", [SegmentOffset - PrevOffset]),
152-
file:write(Fout, Padding);
153-
false ->
154-
throw(
155-
io_lib:format(
156-
"Error: insufficient space for segment ~p. Over by: ~p bytes~n",
157-
[
158-
maps:get(name, Segment), PrevOffset - SegmentOffset
159-
]
160-
)
161-
)
162-
end
163-
end,
164-
SegmentPaths = [
165-
replace(
166-
"BOOT_FILE", BootFile, replace("ROOT_DIR", RootDir, SegmentPath)
138+
try
139+
write_segments(RootDir, BootFile, Fout, Segments)
140+
after
141+
file:close(Fout)
142+
end;
143+
{error, Reason} ->
144+
throw(io_lib:format("Failed to open ~s for writing. Reason: ~p", [OutputFile, Reason]))
145+
end.
146+
147+
%% @private
148+
write_segments(_RootDir, _BootFile, Fout, []) ->
149+
ok;
150+
write_segments(RootDir, BootFile, Fout, [Segment | Segments]) ->
151+
SegmentOffset = from_hex(maps:get(offset, Segment)),
152+
SegmentPaths = [
153+
replace(
154+
"BOOT_FILE", BootFile, replace("ROOT_DIR", RootDir, SegmentPath)
155+
)
156+
|| SegmentPath <- maps:get(path, Segment)
157+
],
158+
SegmentEnd =
159+
case try_read(SegmentPaths) of
160+
{ok, Data} ->
161+
case file:write(Fout, Data) of
162+
ok ->
163+
ok;
164+
{error, WriteError} ->
165+
Fmt = "Failed to write segment data from ~p to image. Reason: ~p.",
166+
throw(io_lib:format(Fmt, [SegmentPaths, WriteError]))
167+
end,
168+
io:format("Wrote ~s (~p bytes) at offset ~s (~p)~n", [
169+
maps:get(name, Segment),
170+
byte_size(Data),
171+
maps:get(offset, Segment),
172+
SegmentOffset
173+
]),
174+
SegmentOffset + byte_size(Data);
175+
{error, Reason} ->
176+
Fmt =
177+
"Failed to read file ~p Reason: ~p."
178+
" Note that a full build is required before running this command.",
179+
throw(io_lib:format(Fmt, [SegmentPaths, Reason]))
180+
end,
181+
if
182+
Segments == [] ->
183+
%% Pad to end on 32-byte alignment, since we don't have a next offset to pad to.
184+
SectorPad = (32 - (SegmentEnd rem 32)) rem 32,
185+
pad_to_size(Fout, SectorPad);
186+
true ->
187+
[PeekNext | _] = Segments,
188+
NextOffset = from_hex(maps:get(offset, PeekNext)),
189+
case SegmentEnd > NextOffset of
190+
true ->
191+
throw(
192+
io_lib:format(
193+
"Error: insufficient space for segment ~p. Overflows partition by: ~p bytes~n",
194+
[maps:get(name, Segment), SegmentEnd - NextOffset]
167195
)
168-
|| SegmentPath <- maps:get(path, Segment)
169-
],
170-
case try_read(SegmentPaths) of
171-
{ok, Data} ->
172-
file:write(Fout, Data),
173-
io:format("Wrote ~s (~p bytes) at offset ~s (~p)~n", [
174-
maps:get(name, Segment),
175-
byte_size(Data),
176-
maps:get(offset, Segment),
177-
SegmentOffset
178-
]),
179-
SegmentOffset + byte_size(Data);
180-
{error, Reason} ->
181-
Fmt =
182-
"Failed to read file ~p Reason: ~p."
183-
" Note that a full build is required before running this command.",
184-
throw(io_lib:format(Fmt, [SegmentPaths, Reason]))
196+
);
197+
false ->
198+
PadSize = NextOffset - SegmentEnd,
199+
case PadSize of
200+
0 ->
201+
ok;
202+
PadSize ->
203+
ok = pad_to_size(Fout, PadSize)
185204
end
186-
end,
187-
undefined,
188-
Segments
189-
);
205+
end
206+
end,
207+
write_segments(RootDir, BootFile, Fout, Segments).
208+
209+
%% @private
210+
pad_to_size(Fout, PadSize) ->
211+
Padding = [16#FF || _ <- lists:seq(1, PadSize)],
212+
case file:write(Fout, Padding) of
213+
ok ->
214+
io:format("Padded ~p bytes~n", [PadSize]);
190215
{error, Reason} ->
191-
throw(io_lib:format("Failed to open ~s for writing. Reason: ~p", [OutputFile, Reason]))
216+
Fmt = "Failed to add ~p bytes padding. Reason: ~p.",
217+
throw(io_lib:format(Fmt, [PadSize, Reason]))
192218
end.
193219

194220
%% @private

0 commit comments

Comments
 (0)