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: crash caused by static variable destructuring #2427

Merged
merged 1 commit into from
Nov 23, 2024

Conversation

Johnson-zs
Copy link
Contributor

Move prefixMap and prefixKeys from static variables to const member variables in EventPrivate class to avoid static initialization/destruction order issues and ensure thread safety.

Log: fix crash

Move prefixMap and prefixKeys from static variables to const member variables
in EventPrivate class to avoid static initialization/destruction order issues
and ensure thread safety.

Log: fix crash
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码重复:在Event::eventType函数中,prefixMapprefixKeys的定义被移到了EventPrivate类中,这是一个好的做法,减少了代码重复。但是,在Event::registerEventType函数中,这些变量仍然被定义,应该移除以保持一致性。

  2. 锁的使用:在Event::eventType函数中,使用了QReadLocker来保护对d->rwLock的读取操作。这是一个好的做法,确保了线程安全。但是,如果d->prefixKeysd->prefixMap的访问是频繁的,可以考虑使用QReadLocker来减少锁的粒度。

  3. 断言的使用:在Event::eventType函数中,使用了Q_ASSERT(splits.size() > 0);来确保topic至少包含一个下划线。这是一个好的做法,可以捕获潜在的错误。但是,如果这个断言失败,程序会立即终止,这可能不是最佳的用户体验。可以考虑添加错误处理逻辑,而不是直接终止程序。

  4. 字符串拼接:在Event::eventType函数中,使用了QString key { space + ":" + topic };来拼接spacetopic。这是一个好的做法,但是可以考虑使用QString::arg方法来提高代码的可读性。

  5. 前缀映射的访问:在Event::eventType函数中,使用了d->prefixMap.value(prefix)来获取EventStratege。这是一个好的做法,但是应该检查prefix是否存在于prefixMap中,以避免潜在的未定义行为。

综合以上意见,代码可以进一步优化,以提高代码的可读性、可维护性和性能。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs, max-lvs

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

@Johnson-zs
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 2a213b9 into linuxdeepin:release/eagle Nov 23, 2024
20 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