Skip to content

feat(lastore-daemon): add SetShutdownForceUpdate D-Bus method for intranet shutdown control#327

Merged
zhaohuiw42 merged 1 commit intolinuxdeepin:masterfrom
zhaohuiw42:master
Mar 12, 2026
Merged

feat(lastore-daemon): add SetShutdownForceUpdate D-Bus method for intranet shutdown control#327
zhaohuiw42 merged 1 commit intolinuxdeepin:masterfrom
zhaohuiw42:master

Conversation

@zhaohuiw42
Copy link
Copy Markdown
Contributor

No description provided.

@github-actions
Copy link
Copy Markdown

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴,但您似乎并非合作伙伴的成员或对接人,请联系相关对接人将您添加至组织之中,或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

You can retrigger this bot by commenting recheck in this Pull Request

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码主要是在 D-Bus 接口中新增了一个 SetShutdownForceUpdate 方法,用于在关机时强制更新。以下是对这段代码的审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面的改进建议:

1. 语法逻辑

  • 权限检查顺序问题:当前代码先检查 m.configm.config.IntranetUpdate,然后才检查调用者的权限(checkInvokePermission)。建议先进行权限检查,因为权限验证通常应该在任何业务逻辑之前进行,以避免未授权用户获取任何系统信息或执行任何操作。
  • 返回值处理m.statusManager.SetFrontForceUpdate(force) 的返回值被忽略了。如果该方法可能返回错误,应该处理该错误。

2. 代码质量

  • 缺少注释:函数 SetShutdownForceUpdate 缺少注释,建议添加函数注释说明其用途、参数含义和返回值说明。
  • 魔法值IntranetUpdate 的含义不够明确,建议将其重命名为更具描述性的名称,如 EnableIntranetUpdateAllowIntranetUpdate
  • 错误处理:如果 m.statusManager 可能为 nil,应该添加检查。

3. 代码性能

  • 锁的使用:虽然这段代码中没有显式使用锁,但 m.service.DelayAutoQuit() 可能会涉及内部锁。如果 SetShutdownForceUpdate 会被频繁调用,需要确保不会造成性能瓶颈。
  • 配置检查m.config.IntranetUpdate 的检查在每次调用时都会执行,如果该配置不会频繁变化,可以考虑缓存其值。

4. 代码安全

  • 权限验证checkInvokePermission 的实现未在代码中展示,但需要确保它验证了调用者是否有权限执行此操作。建议确保该方法足够严格,例如检查调用者是否为 root 或具有特定权限的用户。
  • 配置检查m.config.IntranetUpdate 的检查逻辑可能导致信息泄露。如果未满足条件,函数直接返回 nil,调用者可能无法区分是权限不足还是配置未启用。建议返回明确的错误信息。
  • 参数验证force 参数是布尔值,无需额外验证。但如果未来扩展为其他类型(如字符串或整数),需要添加验证逻辑。

改进后的代码示例:

// SetShutdownForceUpdate 设置关机时是否强制更新
// 参数:
//   - sender: D-Bus 调用者标识
//   - force: 是否强制更新
// 返回值:
//   - *dbus.Error: 操作成功返回 nil,否则返回错误信息
func (m *Manager) SetShutdownForceUpdate(sender dbus.Sender, force bool) *dbus.Error {
    // 先检查权限
    err := checkInvokePermission(m.service, sender)
    if err != nil {
        return dbusutil.ToError(err)
    }

    // 检查配置
    if m.config == nil || !m.config.EnableIntranetUpdate {
        return dbusutil.ToError(errors.New("intranet update is not enabled"))
    }

    m.service.DelayAutoQuit()

    // 检查 statusManager 是否可用
    if m.statusManager == nil {
        return dbusutil.ToError(errors.New("status manager is not available"))
    }

    // 处理错误
    if err := m.statusManager.SetFrontForceUpdate(force); err != nil {
        return dbusutil.ToError(err)
    }

    return nil
}

总结

  • 优点:代码结构清晰,逻辑简单,符合 Go 语言的基本规范。
  • 改进点:建议调整权限检查顺序,添加错误处理和注释,确保配置检查的安全性,并优化性能(如果需要)。
  • 安全性:确保权限验证足够严格,避免信息泄露,并处理可能的错误情况。

以上建议旨在提高代码的健壮性、可维护性和安全性。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiuzhiqian, zhaohuiw42

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

@zhaohuiw42 zhaohuiw42 merged commit 1d37d31 into linuxdeepin:master Mar 12, 2026
14 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