Skip to content

Fix: Remove useless code.#618

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/eaglefrom
GongHeng2017:202603171030-dev-eagle-fix
Mar 17, 2026
Merged

Fix: Remove useless code.#618
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/eaglefrom
GongHeng2017:202603171030-dev-eagle-fix

Conversation

@GongHeng2017
Copy link
Copy Markdown
Contributor

@GongHeng2017 GongHeng2017 commented Mar 17, 2026

-- Remove useless code.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-352671.html

Summary by Sourcery

Enhancements:

  • Clean up CommonTools by removing the unused getGpuInfoCommandFromDConfig helper and its declaration.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Mar 17, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Removes an unused helper method for retrieving GPU info from DConfig from CommonTools, simplifying the GPU info utilities API.

Class diagram for CommonTools after removing getGpuInfoCommandFromDConfig

classDiagram
    class QObject

    class CommonToolsBefore {
        +static QString getBackupPath()
        +static void parseEDID(QStringList allEDIDS, QString input, bool isHW)
        +static QString getGpuInfoCommandFromDConfig()
        +static QString preGenerateGpuInfo()
        +static bool hasPciGraphicsCard()
    }

    class CommonToolsAfter {
        +static QString getBackupPath()
        +static void parseEDID(QStringList allEDIDS, QString input, bool isHW)
        +static QString preGenerateGpuInfo()
        +static bool hasPciGraphicsCard()
    }

    QObject <|-- CommonToolsBefore
    QObject <|-- CommonToolsAfter
Loading

File-Level Changes

Change Details Files
Remove unused GPU info DConfig helper to clean up CommonTools API.
  • Deleted the getGpuInfoCommandFromDConfig() implementation that created a DConfig object and read the CommandToGetGPUInfo key.
  • Removed the getGpuInfoCommandFromDConfig() declaration from the CommonTools class interface.
deepin-devicemanager/src/Tool/commontools.cpp
deepin-devicemanager/src/Tool/commontools.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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码的修改主要是删除了 getGpuInfoCommandFromDConfig 函数。以下是对这段 diff 的审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面的分析:

1. 语法逻辑

  • 审查结果:通过
  • 分析:删除操作本身在语法上没有问题。移除了函数定义(.cpp)和声明(.h),逻辑上保持了一致性,没有留下悬空的引用。

2. 代码质量

  • 审查结果:需改进
  • 分析
    • 资源泄漏风险(历史遗留):虽然函数已被删除,但回顾原代码,存在一个潜在的内存泄漏问题。DConfig::create 返回了一个指针,但代码中并未对其进行 delete 操作。
    • 改进建议:如果在未来的代码中重新引入类似功能,建议使用 QScopedPointerstd::unique_ptr 来管理 DConfig 对象的生命周期,或者确认 DConfig::create 返回的对象是否由框架自动管理(例如通过 QObject 的父子关系)。
    • 示例
      // 假设 DConfig 继承自 QObject
      DConfig *dconfig = DConfig::create("org.deepin.devicemanager", "org.deepin.devicemanager");
      dconfig->setParent(this); // 如果在类成员函数中,或者设置合适的父对象
      // 或者使用智能指针
      QScopedPointer<DConfig> dconfig(DConfig::create(...));

3. 代码性能

  • 审查结果:通过
  • 分析:删除该函数减少了代码量,消除了不必要的函数调用开销,对性能有轻微的正面影响(尽管该函数可能不常被调用)。原函数中涉及 DConfig 的创建和字符串操作,删除后避免了这些开销。

4. 代码安全

  • 审查结果:通过
  • 分析
    • 配置注入风险:原函数从 DConfig 中读取命令字符串(CommandToGetGPUInfo)。如果该配置项可被外部修改(例如通过配置文件或 DBus),攻击者可能注入恶意命令。删除该函数消除了这一潜在的安全风险。
    • 输入验证缺失(历史遗留):原函数未对读取到的命令字符串进行验证或过滤。如果必须保留类似功能,应增加白名单验证,确保只允许预定义的安全命令执行。

总结

  • 主要问题:原函数存在资源泄漏风险(未释放 DConfig 对象)和潜在的安全风险(命令注入)。
  • 建议:当前的删除操作是合理的,消除了潜在的安全隐患。如果未来需要重新实现类似功能,务必注意资源管理和输入验证。

补充建议

如果该功能确实被移除,请确保:

  1. 没有其他代码依赖 getGpuInfoCommandFromDConfig 函数。
  2. 相关的测试用例也已更新或移除。
  3. 文档(如有)已同步更新,反映该功能的移除。

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 reviewed your changes and they look great!


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.

@github-actions
Copy link
Copy Markdown

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "deepin-devicemanager/src/Tool/commontools.cpp": [
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 165,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 169,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://drive-pre.uniontech.com/api/v1/drive/search\";",
            "line_number": 171,
            "rule": "S35",
            "reason": "Url link | 0e010283c1"
        }
    ]
}

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos, GongHeng2017, max-lvs

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

@GongHeng2017
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented Mar 17, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit 7a1a61a into linuxdeepin:develop/eagle Mar 17, 2026
19 of 22 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