refactor: replace raw pointer with unique_ptr in NotificationConsumer - #1212
refactor: replace raw pointer with unique_ptr in NotificationConsumer#1212xq9mend wants to merge 1 commit into
Conversation
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
9a9cadf to
8ab7f45
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
8ab7f45 to
54bb184
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
54bb184 to
1ee2c80
Compare
|
/azpw run Azure.sonic-swss-common |
|
Retrying failed(or canceled) jobs... |
|
No Azure DevOps builds found for #1212. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1ee2c80 to
ccfec8b
Compare
|
/azpw run Azure.sonic-swss-common |
|
/azp run |
|
Retrying failed(or canceled) jobs... |
|
No Azure DevOps builds found for #1212. |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
|
/azpw run Azure.sonic-swss-common |
|
Retrying failed(or canceled) jobs... |
|
Retrying failed(or canceled) stages in build 1144141: ✅Stage BuildSairedis:
|
|
/azpw run Azure.sonic-swss-common |
|
Retrying failed(or canceled) jobs... |
|
Retrying failed(or canceled) stages in build 1144141: ✅Stage BuildSwss:
|
|
/azp run Azure.sonic-swss-common |
|
Commenter does not have sufficient privileges for PR 1212 in repo sonic-net/sonic-swss-common |
|
/azp run Azure.sonic-swss-common |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
m_subscribe was a raw DBConnector* requiring manual delete in two places: the destructor and the constructor's catch block. This is error-prone and exception-unsafe. Replace with std::unique_ptr<DBConnector>: - Remove explicit destructor (compiler-generated handles cleanup) - Remove delete in catch block (reset is automatic on reassignment) - Remove NULL initializer (unique_ptr default-constructs to nullptr) - Use std::make_unique<DBConnector>() in subscribe() - Add .get() where raw pointer is passed to RedisReply constructor Test: add SubscribeViaUnixSocket to ntf_ut.cpp to cover the unix-socket branch of subscribe() (all previous tests used TCP connections). Signed-off-by: xq9mend <xq9mend@users.noreply.github.com>
ccfec8b to
b5b4c90
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Refactors NotificationConsumer subscription ownership to use RAII and adds Unix-socket coverage.
Changes:
- Replaces raw
DBConnector*ownership withstd::unique_ptr. - Removes manual deletion and uses
std::make_unique. - Tests Unix-socket notification subscription.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
common/notificationconsumer.h |
Defines unique ownership and default cleanup. |
common/notificationconsumer.cpp |
Implements automatic connector allocation and cleanup. |
tests/ntf_ut.cpp |
Verifies Unix-socket subscription and delivery. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
What
Replace raw
DBConnector*memberm_subscribewithstd::unique_ptr<DBConnector>inNotificationConsumer.Why
The raw pointer required manual
deletein two separate places — the destructor and the constructor catch block. This pattern is error-prone and exception-unsafe: if a future change adds an early return or throws between allocation and cleanup, the pointer leaks.How
unique_ptrdelete m_subscribefrom catch block — automatic on reassignmentNULLinitializer —unique_ptrdefault-constructs tonullptrnew DBConnector(...)withstd::make_unique<DBConnector>(...)insubscribe().get()where raw pointer is passed toRedisReplyconstructor<memory>include to both.cppand.h