Revert "chore: temporary disable TreelandUserConfig smart pointer"#678
Merged
zccrs merged 1 commit intolinuxdeepin:masterfrom Jan 29, 2026
Merged
Revert "chore: temporary disable TreelandUserConfig smart pointer"#678zccrs merged 1 commit intolinuxdeepin:masterfrom
zccrs merged 1 commit intolinuxdeepin:masterfrom
Conversation
This reverts commit 779832f.
|
Skipping CI for Draft Pull Request. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRe-wraps Helper::m_config in a std::unique_ptr, updating its initialization, reassignment, and accessor to use smart-pointer semantics instead of raw pointers. Class diagram for Helper using std_unique_ptr_TreelandUserConfigclassDiagram
class Helper {
- static Helper* m_instance
- std_unique_ptr~TreelandUserConfig~ m_config
- std_unique_ptr~TreelandConfig~ m_globalConfig
- Treeland_Treeland* m_treeland
- FpsDisplayManager* m_fpsManager
+ Helper(QObject* parent)
+ static Helper* instance()
+ TreelandUserConfig* config()
+ TreelandConfig* globalConfig()
+ void init(Treeland_Treeland* treeland)
}
class TreelandUserConfig {
+ static TreelandUserConfig* createByName(QString serviceName, QString interfaceName, QString path)
}
class TreelandConfig {
+ static std_unique_ptr~TreelandConfig~ create(QString serviceName, QString interfaceName)
}
class Treeland_Treeland
class FpsDisplayManager
class QObject
Helper --> TreelandUserConfig : owns via std_unique_ptr
Helper --> TreelandConfig : owns via std_unique_ptr
Helper --> Treeland_Treeland
Helper --> FpsDisplayManager
Helper --> QObject : inherits
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
TAG Bot New tag: 0.8.1 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/seat/helper.cpp:215-220` </location>
<code_context>
Q_ASSERT(!m_config);
- m_config = TreelandUserConfig::createByName("org.deepin.dde.treeland.user",
+ m_config.reset(TreelandUserConfig::createByName("org.deepin.dde.treeland.user",
"org.deepin.dde.treeland",
- "/dde"); // will update user path in Helper::init
</code_context>
<issue_to_address>
**issue (bug_risk):** Resetting the unique_ptr may invalidate external raw pointers to the config.
With the raw pointer, reassigning `m_config` leaked the old instance but didn’t invalidate any cached raw pointers. Using `std::unique_ptr::reset()` now correctly deletes the old config, so any code that has stored a `Helper::config()` pointer will end up with a dangling pointer after `updateCurrentUser`. Callers should not cache this pointer long-term, or `config()` should expose a safer API (e.g., reference or handle) that makes ownership and lifetime explicit.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+215
to
220
| m_config.reset(TreelandUserConfig::createByName("org.deepin.dde.treeland.user", | ||
| "org.deepin.dde.treeland", | ||
| "/dde"); // will update user path in Helper::init | ||
| "/dde")); // will update user path in Helper::init | ||
| m_globalConfig.reset(TreelandConfig::create("org.deepin.dde.treeland", | ||
| QString())); | ||
|
|
There was a problem hiding this comment.
issue (bug_risk): Resetting the unique_ptr may invalidate external raw pointers to the config.
With the raw pointer, reassigning m_config leaked the old instance but didn’t invalidate any cached raw pointers. Using std::unique_ptr::reset() now correctly deletes the old config, so any code that has stored a Helper::config() pointer will end up with a dangling pointer after updateCurrentUser. Callers should not cache this pointer long-term, or config() should expose a safer API (e.g., reference or handle) that makes ownership and lifetime explicit.
zccrs
approved these changes
Jan 29, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wineee, zccrs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This reverts commit 779832f.
Summary by Sourcery
Enhancements: