Skip to content

fix(enable): use sysfs to detect wireless NIC instead of name prefix#668

Open
GongHeng2017 wants to merge 1 commit into
linuxdeepin:develop/eaglefrom
GongHeng2017:202605211910-eagle-fix
Open

fix(enable): use sysfs to detect wireless NIC instead of name prefix#668
GongHeng2017 wants to merge 1 commit into
linuxdeepin:develop/eaglefrom
GongHeng2017:202605211910-eagle-fix

Conversation

@GongHeng2017
Copy link
Copy Markdown
Contributor

@GongHeng2017 GongHeng2017 commented May 21, 2026

Replace hardcoded name prefix matching (wlan/wlp) with sysfs path checking (/sys/class/net//wireless and phy80211) for more reliable wireless NIC detection.

使用 sysfs 路径检测替代硬编码名称前缀匹配来判断无线网卡,提升检测准确性。

Log: 修复无线网卡检测逻辑,使用sysfs替代名称前缀匹配
PMS: https://pms.uniontech.com/bug-view-362409.html
Influence: 无线网卡启用/禁用操作不再依赖接口名称前缀,支持所有符合内核规范的无线网卡接口名称。

Summary by Sourcery

Detect wireless network interfaces via sysfs paths instead of hardcoded name prefixes when enabling or disabling devices.

Bug Fixes:

  • Fix wireless NIC detection so it no longer relies on interface name prefixes like wlan/wlp but uses kernel-provided sysfs indicators instead.

Enhancements:

  • Add a reusable helper to classify network interfaces as wireless based on /sys/class/net entries.
  • Update file copyright years to cover up to 2026.

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017

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

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 21, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors wireless network interface detection in EnableUtils to rely on sysfs entries instead of hardcoded interface name prefixes, and encapsulates the new detection logic in a private helper while updating metadata headers.

File-Level Changes

Change Details Files
Replace wireless NIC detection based on interface name prefixes with sysfs-based detection and encapsulate it in a helper method.
  • Replace direct logicalName.startsWith("wlan"/"wlp") condition with a call to a new isWireless helper in ioctlOperateNetworkLogicalName
  • Add inclusion of QFileInfo to support filesystem-based checks
  • Implement static private isWireless method to test for /sys/class/net//wireless or phy80211 path existence
  • Declare isWireless as a private static method in the EnableUtils class header
deepin-devicemanager-server/deepin-devicecontrol/src/enablecontrol/enableutils.cpp
deepin-devicemanager-server/deepin-devicecontrol/src/enablecontrol/enableutils.h
Update SPDX copyright year range metadata.
  • Update SPDX-FileCopyrightText year range from 2019 ~ 2023 to 2019 - 2026 in implementation file
  • Update SPDX-FileCopyrightText year range from 2019 ~ 2023 to 2019 - 2026 in header file
deepin-devicemanager-server/deepin-devicecontrol/src/enablecontrol/enableutils.cpp
deepin-devicemanager-server/deepin-devicecontrol/src/enablecontrol/enableutils.h

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

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.

Hey - I've left some high level feedback:

  • The isWireless helper can be simplified to a single return statement (e.g., return wirelessInfo.exists() || phyInfo.exists();) to avoid the explicit if/else and keep the code concise.
  • Consider using QFile::exists() directly instead of constructing QFileInfo objects in isWireless, since you only need to check existence and not other file metadata.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `isWireless` helper can be simplified to a single return statement (e.g., `return wirelessInfo.exists() || phyInfo.exists();`) to avoid the explicit `if/else` and keep the code concise.
- Consider using `QFile::exists()` directly instead of constructing `QFileInfo` objects in `isWireless`, since you only need to check existence and not other file metadata.

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.

Replace hardcoded name prefix matching (wlan/wlp) with sysfs path
checking (/sys/class/net/<name>/wireless and phy80211) for more
reliable wireless NIC detection.

使用 sysfs 路径检测替代硬编码名称前缀匹配来判断无线网卡,提升检测准确性。

Log: 修复无线网卡检测逻辑,使用sysfs替代名称前缀匹配
PMS: https://pms.uniontech.com/bug-view-362409.html
Influence: 无线网卡启用/禁用操作不再依赖接口名称前缀,支持所有符合内核规范的无线网卡接口名称。
@GongHeng2017 GongHeng2017 force-pushed the 202605211910-eagle-fix branch from 96b9844 to 82e248f Compare May 22, 2026 09:28
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码提交的主要目的是优化无线网卡禁用/启用的逻辑,将原来通过正则解析 rfkill list 命令输出的方式,改为直接读取 Linux sysfs 虚拟文件系统(/sys/class/ieee80211//sys/class/net/)的方式。这是一个非常好的改进,避免了复杂的文本解析,且不受系统语言环境的影响。

以下是对该 diff 的详细代码审查,涵盖语法逻辑、代码质量、代码性能和代码安全四个方面:

一、 语法与逻辑

  1. 逻辑漏洞:rfkillId 为空时的执行流
    ioctlOperateNetworkLogicalName 函数中,当读取不到 rfkillId 时,代码仅输出 qCritical() << "Empty rfkill ID";,但并未 return false;。这会导致程序继续往下执行第四步“执行 ifconfig up/down”。如果在没有获取到 rfkillId 的情况下执行网络操作,可能会导致设备状态不一致(例如:硬件层面被 block,但软件层面被 up)。
    改进意见:在输出日志后增加 return false;

  2. 逻辑漏洞:QProcess::start 缺少等待启动和错误状态判断
    第一步中 iwProcess.start("iw", ...) 后直接调用了 waitForFinished()。如果系统没有安装 iw 命令,或者启动失败,waitForFinished() 会立刻返回 false,但代码没有检查返回值,也没有检查进程的退出码。此时 readAllStandardOutput() 将为空,后续的正则匹配必然失败,但开发者很难立刻定位是因为 iw 命令不存在导致的。
    改进意见:检查 waitForFinished() 的返回值及进程退出码,失败时给出明确日志并 return false;

  3. 潜在逻辑缺陷:多 rfkill 设备处理
    代码中使用 phyDir.entryList(...).first() 获取第一个 rfkill 目录。在某些复杂的硬件环境下,一个 phy 设备可能会关联多个 rfkill 设备(虽然罕见)。直接取第一个可能在边缘情况下行为不符合预期。
    改进意见:当前逻辑在绝大多数情况下是正确的,但建议增加一行日志,输出实际读取到的 rfkill 目录名,便于排查问题:qDebug() << "Using rfkill dir:" << rfkillDirs.first();

二、 代码质量

  1. 拼写错误

    • qCritical() << "No rfkill directory found unbder" << phyPath;
      改进意见unbder 应为 under
    • qCritical() << "Failded to read rfkill index from" << rfkillIndexPath;
      改进意见Failded 应为 Failed
  2. 代码简洁性
    新增的 isWireless 函数中:

    if (wirelessInfo.exists() || phyInfo.exists())
        return true;
    else
        return false;

    这种 if (condition) return true else return false 的写法较为冗余。
    改进意见:可以直接返回条件表达式的结果:

    return wirelessInfo.exists() || phyInfo.exists();
  3. 文件关闭的冗余操作
    indexFile.readLine() 之后调用了 indexFile.close();。虽然显式关闭是个好习惯,但在 Qt 中,QFile 离开作用域时其析构函数会自动关闭文件。如果采用更现代的 C++ RAII 思想,可以省略这行代码;如果保留,也不算错,但保持代码风格统一即可。

三、 代码性能

  1. isWireless 函数的系统调用开销
    isWireless 通过检查 /sys/class/net/... 路径是否存在来判断是否为无线网卡,这比原来硬编码判断 wlan/wlp 前缀要准确得多。虽然 QFileInfo::exists() 会引入一次 stat 系统调用,但考虑到这是设备管理操作(非高频调用),性能损耗完全可以接受。这是一个正向优化

  2. 减少了进程创建开销
    原代码需要启动 rfkill list 进程并读取其标准输出,新代码改为直接读取 sysfs 文件。进程的创建与销毁开销远大于文件 I/O。这也是一个显著的正向优化

四、 代码安全

  1. 命令注入风险(低)
    logicalName 被直接传入 iwProcess.start("iw", QStringList() << "dev" << logicalName << "info");。因为使用的是重载的 start(command, arguments) 形式,而不是 start(QString("iw dev %1 info").arg(logicalName)),所以不存在 Shell 注入风险,这是安全的。不过,仍需确保 logicalName 在上层被正确校验(如不包含路径分隔符等),防止恶意构造的设备名引发异常。

  2. 符号链接安全(低)
    /sys/class/net//sys/class/ieee80211/ 下的文件大多是内核提供的符号链接,指向真实的 sysfs 设备目录。QFileInfo::exists()QDir::entryList() 默认会跟随符号链接。在 sysfs 这种由内核管控的虚拟文件系统中,跟随符号链接是安全的,不存在恶意指向的问题。


改进后的代码建议

针对以上审查意见,修改后的核心代码部分如下:

// enableutils.cpp

bool EnableUtils::ioctlOperateNetworkLogicalName(const QString &logicalName, bool enable)
{
    if (isWireless(logicalName)) {  // Wireless LAN
        // 第一步:获取 wiphy 编号
        QProcess iwProcess;
        iwProcess.start("iw", QStringList() << "dev" << logicalName << "info");
        if (!iwProcess.waitForFinished()) {
            qCritical() << "Failed to execute 'iw' command or timed out.";
            return false;
        }
        if (iwProcess.exitCode() != 0) {
            qCritical() << "'iw' command returned error:" << iwProcess.readAllStandardError();
            return false;
        }

        QString iwOutput = QString::fromUtf8(iwProcess.readAllStandardOutput());
        QRegularExpression wiphyRe("wiphy (\\d+)");
        QRegularExpressionMatch wiphyMatch = wiphyRe.match(iwOutput);
        if (!wiphyMatch.hasMatch()) {
            qCritical() << "Failed to parse wiphy number from iw output.";
            return false;
        }
        QString phyNum = wiphyMatch.captured(1);

        // 第二步:获取 rfkill 设备编号
        QString rfkillId;
        QString phyPath = QString("/sys/class/ieee80211/phy%1").arg(phyNum);
        QDir phyDir(phyPath);
        if (!phyDir.exists()) {
            qCritical() << "Phy path does not exist:" << phyPath;
            return false;
        }

        // 查找 rfkill 目录
        QStringList rfkillDirs = phyDir.entryList(QStringList() << "rfkill*", QDir::Dirs);
        if (rfkillDirs.isEmpty()) {
            qCritical() << "No rfkill directory found under" << phyPath; // 修复拼写 unbder -> under
            return false;
        }

        // 读取 index 文件
        QString rfkillIndexPath = phyPath + "/" + rfkillDirs.first() + "/index";
        QFile indexFile(rfkillIndexPath);
        if (indexFile.open(QIODevice::ReadOnly | QIODevice::Text)) {
            rfkillId = indexFile.readLine().trimmed();
            indexFile.close();
        } else {
            qCritical() << "Failed to read rfkill index from" << rfkillIndexPath; // 修复拼写 Failded -> Failed
            return false;
        }

        // 第三步:执行 rfkill block/unblock
        if (!rfkillId.isEmpty()) {
            QProcess rfkillProcess;
            rfkillProcess.start("rfkill", QStringList() << (enable ? "unblock" : "block") << rfkillId);
            rfkillProcess.waitForFinished();
            int ret = rfkillProcess.exitCode();
            if (ret != 0) {
                qCritical() << "Failed to block/unblock wifi: error code: " << ret;
            }
        } else {
            qCritical() << "Empty rfkill ID";
            return false; // 【关键修复】空 ID 时应终止操作,避免状态不一致
        }

        // 第四步:执行 ifconfig up/down
        // ... (后续代码保持不变)
// isWireless 函数优化
bool EnableUtils::isWireless(const QString &logicalName)
{
    QFileInfo wirelessInfo(QString("/sys/class/net/%1/wireless").arg(logicalName));
    QFileInfo phyInfo(QString("/sys/class/net/%1/phy80211").arg(logicalName));
    // 简化逻辑
    return wirelessInfo.exists() || phyInfo.exists();
}

总结:此次代码重构方向十分正确,通过利用 Linux sysfs 文件系统替代命令行解析,提高了代码的健壮性和可维护性。只需注意修复拼写错误、完善错误处理的闭环(特别是空 rfkillId 时的 return false),以及增加对 iw 命令执行失败的容错处理,代码质量将会更加完善。

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