Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Problem with password change pop-up not displaying #372

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

caixr23
Copy link
Contributor

@caixr23 caixr23 commented Jul 18, 2024

Synchronize Professional Version Code

Issue: linuxdeepin/developer-center#9726

@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • NetworkDialog类的构造函数中新增了m_needIdentitym_quitNow成员变量,但未在构造函数中初始化,可能导致未定义行为。
  • onSecretsResultonPassword方法中使用了Q_UNUSED宏,但没有检查socket是否为nullptr,可能会导致空指针解引用。
  • exit方法在NetworkDialog类中声明,但没有在类中实现,这可能是一个遗漏。
  • readyReadHandler方法中没有处理m_quitNow标志,可能会导致在接收到数据时提前退出。
  • onPassword方法中,如果propsObj.contains(key)false,则没有设置password字段,可能会导致协议不兼容的问题。

是否建议立即修改:

  • 确保所有新增的成员变量在类的构造函数中都有明确的初始化方式,以避免潜在的未定义行为。
  • 检查Q_UNUSED宏的使用,确保socket不为nullptr,以避免潜在的空指针解引用错误。
  • 实现exit方法,以便在NetworkDialog类中正确处理退出操作。
  • readyReadHandler方法中添加对m_quitNow的检查,确保在接收到数据时不会提前退出。
  • 确保onPassword方法的逻辑完整,包括对propsObj.contains(key)的检查,以避免协议不兼容的问题。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, yixinshark

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@caixr23
Copy link
Contributor Author

caixr23 commented Jul 18, 2024

/merge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jul 18, 2024

This pr cannot be merged! (status: unstable)

@caixr23
Copy link
Contributor Author

caixr23 commented Jul 19, 2024

/merge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jul 19, 2024

This pr cannot be merged! (status: unstable)

@yixinshark yixinshark merged commit 74e905f into linuxdeepin:master Jul 19, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants