Skip to content

fix: add null pointer checks for D-Bus proxy member variables#3255

Open
xionglinlin wants to merge 1 commit into
linuxdeepin:masterfrom
xionglinlin:master
Open

fix: add null pointer checks for D-Bus proxy member variables#3255
xionglinlin wants to merge 1 commit into
linuxdeepin:masterfrom
xionglinlin:master

Conversation

@xionglinlin
Copy link
Copy Markdown
Contributor

  1. Add null pointer checks before accessing properties on m_defaultSink, m_defaultSource, and m_sourceMeter
  2. Return safe default values (false, 0.0, 0, empty AudioPort, empty QDBusObjectPath) when pointers are null
  3. Prevent potential crashes caused by dereferencing null pointers when D-Bus services are unavailable or not properly initialized

Log: null pointer checks added for sound D-Bus proxy members

Influence:

  1. Test sound settings panel with audio device disconnected or unavailable
  2. Verify mute, volume, balance, base volume, active port, and card functions return safe defaults
  3. Test source and meter related functions with null D-Bus object
  4. Ensure no application crashes occur when audio D-Bus services are not ready
  5. Validate audio state transitions between device plugged and unplugged scenarios

fix: 为 D-Bus 代理成员变量添加空指针检查

  1. 在访问 m_defaultSink、m_defaultSource 和 m_sourceMeter 的属性之前添加 空指针检查
  2. 当指针为空时返回安全默认值(false、0.0、0、空 AudioPort、空 QDBusObjectPath)
  3. 防止因 D-Bus 服务不可用或未正确初始化时解引用空指针导致程序崩溃

Log: 为声音 D-Bus 代理成员添加空指针检查

Influence:

  1. 在音频设备断开或不可用时测试声音设置面板
  2. 验证静音、音量、平衡、基础音量、活动端口和卡片功能返回安全默认值
  3. 在 D-Bus 对象为空时测试源和仪表相关函数
  4. 确保音频 D-Bus 服务未就绪时应用程序不会崩溃
  5. 验证设备插入和拔出场景下音频状态的正确转换

Change-Id: Idb74020cc57cbfe5cec931abb422609de16f60b8

1. Add null pointer checks before accessing properties on m_defaultSink,
m_defaultSource, and m_sourceMeter
2. Return safe default values (false, 0.0, 0, empty AudioPort, empty
QDBusObjectPath) when pointers are null
3. Prevent potential crashes caused by dereferencing null pointers when
D-Bus services are unavailable or not properly initialized

Log: null pointer checks added for sound D-Bus proxy members

Influence:
1. Test sound settings panel with audio device disconnected or
unavailable
2. Verify mute, volume, balance, base volume, active port, and card
functions return safe defaults
3. Test source and meter related functions with null D-Bus object
4. Ensure no application crashes occur when audio D-Bus services are
not ready
5. Validate audio state transitions between device plugged and unplugged
scenarios

fix: 为 D-Bus 代理成员变量添加空指针检查

1. 在访问 m_defaultSink、m_defaultSource 和 m_sourceMeter 的属性之前添加
空指针检查
2. 当指针为空时返回安全默认值(false、0.0、0、空 AudioPort、空
QDBusObjectPath)
3. 防止因 D-Bus 服务不可用或未正确初始化时解引用空指针导致程序崩溃

Log: 为声音 D-Bus 代理成员添加空指针检查

Influence:
1. 在音频设备断开或不可用时测试声音设置面板
2. 验证静音、音量、平衡、基础音量、活动端口和卡片功能返回安全默认值
3. 在 D-Bus 对象为空时测试源和仪表相关函数
4. 确保音频 D-Bus 服务未就绪时应用程序不会崩溃
5. 验证设备插入和拔出场景下音频状态的正确转换

Change-Id: Idb74020cc57cbfe5cec931abb422609de16f60b8
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xionglinlin

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

Copy link
Copy Markdown

@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.

Sorry @xionglinlin, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已经仔细审查了你提供的 git diff。本次代码变更的核心是为 m_defaultSinkm_defaultSourcem_sourceMeter 等 DBus 接口指针添加了空指针检查,以防止在对象未初始化或销毁时发生段错误。

这是一个非常好的防御性编程改进!不过,在语法逻辑、代码质量、性能和安全方面,我还有以下几点改进建议:

1. 语法与逻辑

  • GetMeter() 的异步逻辑隐患
    原代码 m_defaultSource->asyncCallWithArgumentList 是一个异步调用,返回的是 QDBusPendingReply。在 Qt 的 DBus 机制中,QDBusPendingReply 需要通过事件循环等待结果就绪后才能提取值。
    问题:当 m_defaultSource 为空时,你直接返回了 QDBusObjectPath()。如果调用方没有处理这种异步未就绪的情况,直接当作同步结果使用,可能会导致逻辑错误。
    建议:确认调用方是否能接受一个空的 QDBusObjectPath 作为异常返回。如果调用方依赖该返回值进行后续同步操作,建议将此方法改为同步调用 callWithArgumentList,或者在返回空值时增加日志警告。

2. 代码质量

  • 代码重复(DRY原则)
    本次修改在 10+ 个函数中重复了 if (pointer) { return ...; } return default_value; 的模式。这显得代码有些臃肿。
    建议:可以考虑使用 C++17 的折叠表达式或模板,或者编写一个私有的辅助模板函数来简化这类操作。例如:

    template<typename T, typename Obj, typename... Args>
    T getPropertySafe(Obj* obj, const char* propName, T defaultValue = T()) {
        if (obj) {
            return qvariant_cast<T>(obj->property(propName));
        }
        return defaultValue;
    }
    
    // 使用时:
    bool SoundDBusProxy::muteSink() {
        return getPropertySafe<bool>(m_defaultSink, "MuteSink", false);
    }
    double SoundDBusProxy::balanceSink() {
        return getPropertySafe<double>(m_defaultSink, "BalanceSink", 0.0);
    }
  • 缺少异常/错误日志
    当这些指针为空时,直接返回默认值会让问题“静默失败”。如果由于 DBus 服务异常导致指针频繁为空,上层业务逻辑会一直读到 0.0false,这很难排查。
    建议:在判空分支中增加 qWarning()qCWarning() 日志输出,便于开发调试。

  • 版权年份修改存疑
    SPDX-FileCopyrightText2024 - 2027 改为了 2024 - 2026。通常情况下,版权结束年份应该是当前年份或未来的年份,将 2027 改为 2026 似乎是一个逆向操作,请确认这是否是笔误。

3. 代码性能

  • 字符串查找开销
    property("MuteSink") 等调用在 Qt 底层会进行字符串匹配查找。虽然在这个场景下(音频属性读取)性能开销可能并不显著,但在高频调用时(如 UI 滑动条拖动触发的 volumeSink 读取),字符串查找会带来一定开销。
    建议:如果这些属性读取函数被高频调用,建议在对象初始化时(如 setSinkDevicePath 中),通过 QMetaObject::indexOfProperty() 缓存属性的索引,后续使用 property(index) 进行读取,这会从 $O(N)$ 的字符串查找变为 $O(1)$ 的索引查找。

4. 代码安全

  • DBus 通信超时与阻塞风险
    虽然你修复了空指针崩溃问题,但 property() 方法在底层实际上是同步的 DBus 调用。如果 DBus 守护进程或音频服务无响应,调用 property() 可能会导致当前线程阻塞,甚至触发 Qt 的默认 DBus 超时警告。
    建议:确保这些函数不在主线程(UI 线程)中被频繁同步调用。如果涉及高频更新,建议全面转向信号/槽机制(监听 DBus 的 PropertiesChanged 信号),而不是主动去拉取属性。

综合改进示例代码

结合以上建议,这里给出一个优化后的代码示例:

// 在头文件中增加模板辅助函数(private)
template<typename T>
T getDbusPropertySafe(QDBusAbstractInterface* interface, const QString& propName, T defaultValue = T()) {
    if (!interface) {
        qCWarning(ddcSoundLog) << "DBus interface is null when querying property:" << propName;
        return defaultValue;
    }
    return qvariant_cast<T>(interface->property(propName.toUtf8().constData()));
}

// CPP 文件实现
bool SoundDBusProxy::muteSink()
{
    return getDbusPropertySafe<bool>(m_defaultSink, "MuteSink", false);
}

double SoundDBusProxy::balanceSink()
{
    return getDbusPropertySafe<double>(m_defaultSink, "BalanceSink", 0.0);
}

QDBusObjectPath SoundDBusProxy::GetMeter()
{
    if (m_defaultSource) {
        QList<QVariant> argumentList;
        return QDBusPendingReply<QDBusObjectPath>(m_defaultSource->asyncCallWithArgumentList(QStringLiteral("GetMeter"), argumentList));
    }
    qCWarning(ddcSoundLog) << "m_defaultSource is null, cannot call GetMeter";
    return QDBusObjectPath();
}

总结:你的核心修改(增加判空)是非常正确且必要的,能够有效提升程序的健壮性。如果能在代码可读性(减少重复代码)、可观测性(增加日志)上再做些优化,这段代码的质量会更高。

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.

2 participants