-
Notifications
You must be signed in to change notification settings - Fork 61
Conversation
@@ -317,24 +318,24 @@ GObjectUniquePtr<OstreeDeployment> OstreeManager::getStagedDeployment() const { | |||
} | |||
|
|||
GObjectUniquePtr<OstreeSysroot> OstreeManager::LoadSysroot(const boost::filesystem::path &path) { | |||
OstreeSysroot *sysroot = nullptr; | |||
OstreeSysroot *sysroot_raw = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use .reset()
to update the value of a GObjectUniquePtr
and forego the sysroot_raw intermediary which can potentially be misused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, apart from the small style comment!
Also rewrap a pointer from ostree_sysroot_new to try make that cleaner. Signed-off-by: Patrick Vacek <[email protected]>
This appears to be a false positive, and valgrind just says it is "possibly" lost anyway. Signed-off-by: Patrick Vacek <[email protected]>
Signed-off-by: Patrick Vacek <[email protected]>
Valgrind gets really confused by this and spits out a bunch of complaints in strange places (such as ReportQueue) as well. Signed-off-by: Patrick Vacek <[email protected]>
8f7347e
to
3bb1beb
Compare
Hm I've taken another look and I'm not sure the ioctl error is exactly the same. Though it's also probably a false positive. Do you have the full valgrind log? |
If I remove the suppression (or comment out the call to
and
|
Codecov Report
@@ Coverage Diff @@
## master #1120 +/- ##
==========================================
- Coverage 76.07% 76.06% -0.02%
==========================================
Files 158 158
Lines 9544 9544
==========================================
- Hits 7261 7260 -1
- Misses 2283 2284 +1
Continue to review full report at Codecov.
|
Ok it's from here: https://github.com/ostreedev/ostree/blob/13bcc49603b54f117c44e25dc2b457b9f25d9dc0/src/libostree/ostree-sysroot-deploy.c#L1375,
But no big deal anyway, this is definitely false positive. |
Actually fix two and suppress two others that look similar to already known issues.