Skip to content

Conversation

@GongHeng2017
Copy link
Contributor

@GongHeng2017 GongHeng2017 commented Aug 1, 2025

-- Code Logic error, "sizeof(logicalName::toStdString().c_str())" always return 8,
not the length of logicalName.
-- So, When the length more than 8, the net card will can not find.

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

Summary by Sourcery

Fix net card wake-on-lan detection for interface names longer than 8 characters by properly initializing wolinfo and correctly copying logicalName into ifr.ifr_name with a fixed-size limit and null termination.

Bug Fixes:

  • Initialize the ethtool_wolinfo struct before use to avoid uninitialized data.
  • Replace incorrect sizeof(logicalName.toStdString().c_str()) with IFNAMSIZ-1 for strncpy and explicitly null-terminate ifr.ifr_name to handle longer interface names.

-- Code Logic error, "sizeof(logicalName::toStdString().c_str())" always return 8,
   not the length of logicalName.
-- So, When the length more than 8, the net card will can not find.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-326565.html
@sourcery-ai
Copy link

sourcery-ai bot commented Aug 1, 2025

Reviewer's Guide

This PR addresses a logic error in interface name copying by replacing the incorrect sizeof-based length on c_str() with a safe copy using IFNAMSIZ-1 and QByteArray conversion, adds explicit zeroing of wolinfo to eliminate uninitialized data, and ensures null termination of ifr.ifr_name in both wakeOnLanIsOpen and setWakeOnLan.

Class diagram for WakeupUtils changes

classDiagram
    class WakeupUtils {
        +EthStatus wakeOnLanIsOpen(const QString &logicalName)
        +bool setWakeOnLan(const QString &logicalName, bool open)
    }
    WakeupUtils : -strncpy(ifr.ifr_name, logicalName.toStdString().c_str(), sizeof(logicalName.toStdString().c_str()))
    WakeupUtils : +QByteArray nameBytes = logicalName.toLocal8Bit()
    WakeupUtils : +strncpy(ifr.ifr_name, nameBytes.constData(), IFNAMSIZ - 1)
    WakeupUtils : +ifr.ifr_name[IFNAMSIZ - 1] = '\0'
    WakeupUtils : +memset(&wolinfo, 0, sizeof(wolinfo))
Loading

File-Level Changes

Change Details Files
Use proper buffer size and conversion for interface names
  • Convert logicalName to a QByteArray via toLocal8Bit()
  • Copy the name into ifr.ifr_name using strncpy with IFNAMSIZ-1
  • Explicitly set the last byte of ifr.ifr_name to '\0' for null-termination
deepin-devicemanager-server/deepin-devicecontrol/src/wakecontrol/wakeuputils.cpp
Initialize wolinfo struct before use
  • Add memset(&wolinfo, 0, sizeof(wolinfo)) at the start of each function
deepin-devicemanager-server/deepin-devicecontrol/src/wakecontrol/wakeuputils.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

代码审查意见:

  1. 代码重复

    • wakeOnLanIsOpensetWakeOnLan 函数中存在大量重复代码,建议提取公共逻辑到一个单独的函数中,以减少代码重复和提高可维护性。
  2. 错误处理

    • ioctl 调用后没有检查返回值是否为0,如果调用失败,应该有相应的错误处理逻辑。
  3. 字符串长度检查

    • 使用 strncpy 时,应该检查 logicalName 的长度是否超过了 IFNAMSIZ - 1,以避免潜在的缓冲区溢出问题。
  4. 内存清理

    • ioctl 调用之前,应该确保 ifrwolinfo 结构体已经被正确初始化,并且在使用完毕后进行清理。
  5. 错误信息

    • 如果 ioctl 调用失败,应该记录或返回错误信息,以便于调试和用户了解错误原因。
  6. 代码注释

    • 代码中缺少必要的注释,特别是对于 ioctl 调用失败时的处理逻辑,应该添加相应的注释以提高代码的可读性。
  7. 线程安全

    • 如果这些函数可能会在多线程环境中被调用,应该确保对共享资源的访问是线程安全的。
  8. 资源管理

    • 如果 fd 是一个文件描述符,应该在使用完毕后关闭它,以避免资源泄露。
  9. 函数命名

    • 函数命名应该更具描述性,例如 wakeOnLanIsOpen 可以改为 isWakeOnLanEnabledsetWakeOnLan 可以改为 enableWakeOnLan
  10. 代码风格

    • 代码风格应该保持一致,例如缩进和空格的使用应该遵循项目的编码规范。

综上所述,建议对代码进行重构,提取公共逻辑,增加错误处理和资源管理,并确保代码风格的一致性。

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

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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
Contributor Author

/merge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Aug 1, 2025

This pr cannot be merged! (status: unstable)

@GongHeng2017
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Aug 1, 2025

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 6961768 into linuxdeepin:develop/eagle Aug 1, 2025
16 of 17 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.

3 participants