Fix memory leak and undefined behavour in Updater.cpp (UpdaterClass) #8671
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
mrengineer7777
approved these changes
Sep 25, 2023
@mrengineer7777 Not sure perhaps targeting |
no. leave it as is. We will not update 2.x anymore and will merge 5.1 into master to focus on 3.0.0 |
VojtechBartoska
added
the
Area: Performance
P-R-O-C-H-Y
reviewed
Oct 4, 2023
There was a problem hiding this comment.
@MathewHDYT When updating this part of code, can you change the error messages as suggested? :) Thanks, otherwise LGTM.
Co-authored-by: Jan Procházka <90197375+P-R-O-C-H-Y@users.noreply.github.com>
Co-authored-by: Jan Procházka <90197375+P-R-O-C-H-Y@users.noreply.github.com>
P-R-O-C-H-Y
approved these changes
Oct 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
The pull request aims to fix undefined behaviour and a memory leak of 16 bytes every time the
UpdaterClassis used, this occurs because the_skipBufferis not freed correctly and the_bufferis deleted but using the C++ styledelete[], even tough the C style memory allocationmallocwas used. Which is not compatible and is undefined behavour. Meaning it might work on some devices on others it might crash and on some it might do other things.The changes are compatible with the current API and do not change anything about the
UpdaterClasspublic or private method. It doesn't even change the private members. It only adjusts the memory allocation to always use C++ stylenewand to actually free the created memory withdelete[].A more detailed description can be found in the issue mentioned below as well.
Tests scenarios
The changed code has been tested with an ESP32 and updating the device with the
UpdaterClassover OTA.Related links
Closes #7984