For linux msm/valgrind fixes #86
Merged
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
ndechesne
reviewed
Dec 17, 2024
util.c
Outdated
| return NULL; | ||
| } |
There was a problem hiding this comment.
an if/else statement would seem slightly better here. nitpicking..
There was a problem hiding this comment.
if/else looks clumsy, the function has the shape "if error early exit, if error early exit, else strdup() and return". But the second "error" isn't really an error, so it looks better turning this into a single-exit.
Thanks.
| @@ -492,7 +492,8 @@ int sahara_run(struct qdl_device *qdl, char *img_arr[], bool single_image, | |||
| } | |||
| } | |||
|
|
|||
| close(ramdump_dir); | |||
| if (ramdump_dir >= 0) | |||
There was a problem hiding this comment.
hygine spelling in the commit message
Every time the USB code attempts to identify and possibly open the EDL USB device, the libusb device descriptor-object is leaked. While at it, also introduce qdl_close() to clean up the overall state, in order to approach not leaking memory in a clean QDL run. Signed-off-by: Bjorn Andersson <[email protected]>
Continuing the journey to make QDL not leak any memory on a successful run, introduce operations to free the program and patch objects on exit. Signed-off-by: Bjorn Andersson <[email protected]>
For each <program/> entry int he list, a scratch buffer of "max payload size" is allocated, and lost. Free this memory instead. Signed-off-by: Bjorn Andersson <[email protected]>
The XML parsing helpers turns libxml strings into standard C-types and leaks the libxml memory. Fix this by freeing the libxml strings once they are accessed. Signed-off-by: Bjorn Andersson <[email protected]>
Valgrind correctly reports that the write of 0x30 bytes accesses uninitialized stack space. Pad the Sahara HELLO request and response with the expected "reserved" entries to reach the defined 0x30 bytes to avoid sending random stack content to the device. Signed-off-by: Bjorn Andersson <[email protected]>
In the event that no ramdump_path is provided to sahara_run() the ramdump_dir will remain -1. For good hygiene avoid calling close() on this invalid file descriptor. Signed-off-by: Bjorn Andersson <[email protected]>
In a few places in firehose.c the memory returned from xmlGetProp() is leaked. Avoid this by freeing it. Signed-off-by: Bjorn Andersson <[email protected]>
andersson
force-pushed
the
for-linux-msm/valgrind-fixes
branch
from
5141e5f to
1a6f38e
Compare
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.
Attempting to debug a bug report with Valgrind highlighted a number of issues in the implementation. The issue turned out to exist elsewhere, but let's resolve all the reported issues, to make Valgrind more useful in detecting future issues - and to improve the quality of the code.