Skip to content

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Jan 19, 2026

Added null pointer check for m_defSinkInter before accessing its properties in removeDisabledDevice method. This prevents potential segmentation faults when the default sink interface is not properly initialized or has been destroyed.

Influence:

  1. Test removing disabled audio devices when m_defSinkInter is null
  2. Verify sound applet stability during device hot-plug operations
  3. Test audio device switching scenarios with multiple sound cards
  4. Verify no crashes occur when disabling audio devices

fix: 在声音小程序中添加对 m_defSinkInter 的空指针检查

在 removeDisabledDevice 方法中访问 m_defSinkInter 属性前添加了空指针检 查。这可以防止当默认接收器接口未正确初始化或已被销毁时可能发生的段错误。

Influence:

  1. 测试当 m_defSinkInter 为空时移除禁用的音频设备
  2. 验证在设备热插拔操作期间声音小程序的稳定性
  3. 测试多声卡场景下的音频设备切换
  4. 验证禁用音频设备时不会发生崩溃

PMS: BUG-347845

Summary by Sourcery

Bug Fixes:

  • Prevent potential crashes in the sound applet by adding a null check before accessing the default sink interface when removing a disabled device.

Added null pointer check for m_defSinkInter before accessing its
properties in removeDisabledDevice method. This prevents potential
segmentation faults when the default sink interface is not properly
initialized or has been destroyed.

Influence:
1. Test removing disabled audio devices when m_defSinkInter is null
2. Verify sound applet stability during device hot-plug operations
3. Test audio device switching scenarios with multiple sound cards
4. Verify no crashes occur when disabling audio devices

fix: 在声音小程序中添加对 m_defSinkInter 的空指针检查

在 removeDisabledDevice 方法中访问 m_defSinkInter 属性前添加了空指针检
查。这可以防止当默认接收器接口未正确初始化或已被销毁时可能发生的段错误。

Influence:
1. 测试当 m_defSinkInter 为空时移除禁用的音频设备
2. 验证在设备热插拔操作期间声音小程序的稳定性
3. 测试多声卡场景下的音频设备切换
4. 验证禁用音频设备时不会发生崩溃

PMS: BUG-347845
@mhduiy mhduiy requested a review from 18202781743 January 19, 2026 09:59
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 19, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds a null check for the default sink interface in the sound applet’s removeDisabledDevice logic to prevent crashes when the default sink is unavailable.

Sequence diagram for updated removeDisabledDevice null-safe behavior

sequenceDiagram
    participant SoundApplet
    participant SoundCardPort
    participant DefSinkInter

    SoundApplet->>SoundCardPort: compositeKey(cardId, portName)
    SoundApplet->>SoundApplet: removePort(compositeKey)
    alt m_defSinkInter is not null
        SoundApplet->>DefSinkInter: activePort()
        DefSinkInter-->>SoundApplet: Port
        SoundApplet->>DefSinkInter: card()
        DefSinkInter-->>SoundApplet: cardId
        alt activePort.name equals portName and card equals cardId
            SoundApplet->>SoundApplet: enableDevice(false)
        else port or card does not match
            SoundApplet->>SoundApplet: do_nothing
        end
    else m_defSinkInter is null
        SoundApplet->>SoundApplet: skip_default_sink_check
    end
Loading

Updated class diagram for SoundApplet removeDisabledDevice logic

classDiagram
    class SoundApplet {
        - DefSinkInter m_defSinkInter
        + enableDevice(flag bool) void
        + removeDisabledDevice(portName QString, cardId unsigned_int) void
        - removePort(compositeKey QString) void
    }

    class SoundCardPort {
        + compositeKey(cardId unsigned_int, portName QString) QString
    }

    class DefSinkInter {
        + activePort() Port
        + card() unsigned_int
    }

    class Port {
        + name QString
    }

    SoundApplet --> DefSinkInter : uses
    SoundApplet --> SoundCardPort : uses
    DefSinkInter --> Port : returns
Loading

File-Level Changes

Change Details Files
Guard access to the default sink interface in removeDisabledDevice with a null check to avoid dereferencing a null pointer.
  • Update the conditional in removeDisabledDevice to first verify m_defSinkInter is non-null before reading activePort().name and card()
  • Keep existing behavior of disabling the device when the default sink matches the removed port and card, now safely guarded
plugins/dde-dock/sound/soundapplet.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码修改主要是为了防止空指针解引用(Null Pointer Dereference)导致的程序崩溃。以下是对该修改的详细审查意见:

1. 语法逻辑审查

  • 修改点分析:在 if 条件判断中增加了 m_defSinkInter 的非空检查。
  • 逻辑正确性:修改后的逻辑是正确的。在访问 m_defSinkInter 的成员函数 activePort()card() 之前,先判断指针本身是否有效。如果 m_defSinkInternullptr,后续的成员访问会导致程序崩溃。
  • 逻辑完整性:该修改确保了在 m_defSinkInter 为空时,不会执行后续的条件判断,从而避免了潜在的崩溃风险。

2. 代码质量审查

  • 防御性编程:这是一个典型的防御性编程实践,通过增加前置条件检查来增强代码的健壮性。
  • 可读性:修改后的代码依然保持了良好的可读性,条件判断清晰明确。
  • 一致性:建议检查项目中其他使用 m_defSinkInter 的地方,确保所有访问该指针的地方都进行了非空检查,以保持代码风格的一致性。

3. 代码性能审查

  • 性能影响:增加一个指针的非空检查对性能的影响微乎其微,因为这是一个简单的指针比较操作,在现代处理器上几乎可以忽略不计。
  • 优化建议:无需进一步优化,当前的性能开销是完全可以接受的。

4. 代码安全审查

  • 安全性提升:该修改显著提高了代码的安全性,防止了因空指针解引用导致的程序崩溃(Segmentation Fault)。
  • 潜在风险:虽然该修改避免了直接崩溃,但需要考虑 m_defSinkInter 为空时的业务逻辑是否正确。例如,是否需要在 m_defSinkInter 为空时执行其他操作(如记录日志或尝试重新初始化)。

5. 改进建议

  1. 日志记录:在 m_defSinkInter 为空时,建议记录日志,以便后续排查问题。例如:
    if (!m_defSinkInter) {
        qWarning() << "m_defSinkInter is null in SoundApplet::removeDisabledDevice";
        return;
    }
  2. 空指针处理:如果 m_defSinkInter 为空是一个异常情况,可以考虑抛出异常或返回错误码,而不是静默处理。
  3. 代码复用:如果 m_defSinkInter 的非空检查在多个地方重复出现,可以封装为一个辅助函数,例如:
    bool SoundApplet::isDefaultSinkValid() const {
        return m_defSinkInter != nullptr;
    }

6. 总结

该修改是一个简单但有效的改进,显著提高了代码的健壮性和安全性。建议结合日志记录和错误处理机制,进一步完善代码的可靠性。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Consider using an early return guard (e.g., if (!m_defSinkInter) return;) at the top of removeDisabledDevice to keep the null check separate and make the condition on activePort().name and card() easier to read.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider using an early return guard (e.g., `if (!m_defSinkInter) return;`) at the top of `removeDisabledDevice` to keep the null check separate and make the condition on `activePort().name` and `card()` easier to read.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, BLumia, mhduiy

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

Details 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

@BLumia BLumia merged commit 1d4969e into linuxdeepin:master Jan 20, 2026
10 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.

4 participants