Skip to content

Commit 9faef56

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. * 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. Also 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 aa7ac0f commit 9faef56

1 file changed

Lines changed: 76 additions & 51 deletions

File tree

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

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

194219
%% @private

0 commit comments

Comments
 (0)