Skip to content

fix: remove "bash -c" in command#275

Merged
zhaohuiw42 merged 1 commit into
linuxdeepin:masterfrom
zhaohuiw42:master
Dec 29, 2025
Merged

fix: remove "bash -c" in command#275
zhaohuiw42 merged 1 commit into
linuxdeepin:masterfrom
zhaohuiw42:master

Conversation

@zhaohuiw42

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Dec 29, 2025

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

我来对这段代码进行详细的审查和分析:

  1. message_report.go 中的改进:
// 修改前:
content, err := base64.RawStdEncoding.DecodeString(c.Shell)

// 修改后:
shell := strings.TrimRight(c.Shell, "=")
content, err := base64.RawStdEncoding.DecodeString(shell)

改进意见:

  • 这个修改是为了处理base64编码中的填充问题,但是需要确保这种修改不会影响其他地方的使用
  • 建议添加注释说明为什么需要去除填充符,以及这种修改的影响范围
  • 可以考虑添加一个专门的函数来处理这种base64解码,而不是内联处理
  1. manager_update.go 中的改进:
// 修改前:
logger.Warning("updatePlatform gen token failed", err)

// 修改后:
logger.Warningf("updatePlatform gen token failed: %v", err)

改进意见:

  • 这个修改很好,使用格式化字符串更符合Go的日志记录习惯
  • 建议统一整个项目的日志格式,保持一致性
  1. check.go 中的改进:
// 修改前:
output, err := runcmd.RunnerOutput(60, "bash", hookPath)

// 修改后:
output, err := runcmd.RunnerOutput(60, hookPath)

改进意见:

  • 这个修改直接执行脚本文件,避免了额外的bash进程开销
  • 需要确保hookPath文件具有可执行权限
  • 建议在执行前检查文件权限
  1. diskinfo.go 中的改进:
// 修改前:
freeSpace, err := runcmd.RunnerOutput(10, "bash", "-c", "df -l --output=avail / | tail -n 1")
// 修改后:
freeSpace, err := runcmd.RunnerOutput(10, "df", "-l", "--output=avail", "/")

改进意见:

  • 这个修改很好,避免了使用bash和管道,直接使用df命令更高效
  • 新增的parseDfAvailOuput函数提高了代码复用性
  • 建议添加更多的错误处理,比如检查输出格式是否符合预期
  1. pkginfo.go 中的改进:
// 修改前:
outputStream, err := runcmd.RunnerOutput(10, "bash", "-c", "dpkg -l | tail -n +6 | awk '{print $1,$2,$3}'")
// 修改后:
outputStream, err := runcmd.RunnerOutput(
    10,
    "dpkg-query",
    "-W",
    "-f=${db:Status-Abbrev} ${Package} ${Version}\n",
)

改进意见:

  • 使用dpkg-query替代管道命令更高效且更可靠
  • 添加了日志记录,便于调试
  • 建议添加更多的输入验证,比如检查包名格式是否合法

总体建议:

  1. 代码整体改进方向是好的,都在提高代码效率和可维护性
  2. 建议统一错误处理方式,使用fmt.Errorf包装错误
  3. 建议添加更多的单元测试来验证这些改动
  4. 对于系统命令的调用,建议添加超时机制和资源限制
  5. 考虑添加更多的日志记录,便于问题追踪和调试

安全建议:

  1. 在执行外部命令时,建议对输入参数进行验证和清理
  2. 对于文件操作,建议检查文件权限和路径合法性
  3. 建议添加命令执行的资源限制,避免资源耗尽
  4. 对于base64解码,建议验证解码后的数据长度是否合理

性能建议:

  1. 避免不必要的bash进程创建
  2. 直接使用系统命令而不是通过管道组合
  3. 合理使用缓存机制,避免重复的系统调用
  4. 考虑并发处理多个操作,提高整体性能

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface, 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 c0f94a3 into linuxdeepin:master Dec 29, 2025
11 of 13 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