Fix: [file-enumerator] use g_file_enumerator_next_file()

[GongHeng2017]

--Replace use of g_file_enumerator_iterate() with g_file_enumerator_next_file() in DEnumerator::hasNext() so the enumerator's internal cursor is advanced by GIO and error/end conditions are handled unambiguously.

--Using g_file_enumerator_next_file() yields clearer semantics: non-NULL -> next entry, NULL + NULL error -> end, NULL + non-NULL error -> error. This avoids premature termination caused by backend-specific iterate() quirks.

--Construct next entry URL via the existing buildUrl(...) helper and preserve existing logic that creates the DFileInfo from the returned GFileInfo. Preserve existing error handling and d->checkFilter() behavior and recursive skipping of filtered entries.

--Fix URL construction so joining directory path and filename does not introduce an extra '/' (previously produced e.g. .../vault_unlocked//file). The logic now concatenates carefully (handles root path and trailing slashes) to avoid duplicate separators.

[deepin-ci-robot]

deepin pr auto review

这份代码修改主要涉及两个函数：buildUrl 和 hasNext。下面是对这两处修改的详细审查，包括语法逻辑、代码质量、性能和安全方面的分析。

1. buildUrl 函数修改分析

修改内容：
将原本的三元运算符逻辑改为 if-else 块，并增加了对路径末尾斜杠的检查。

QString path;
if (url.path() == "/") {
    path = "/" + QString(fileName);
} else {
    QString dirPath = url.path();
    path = dirPath.endsWith('/') ? dirPath + QString(fileName) : dirPath + "/" + QString(fileName);
}

审查意见：

• 语法逻辑：

  • 逻辑正确：修改后的逻辑处理了根路径 / 的情况，同时也处理了非根路径末尾是否有 / 的情况，避免了路径拼接时出现 // 或缺少 / 的问题。
  • 冗余检查：QUrl::path() 在大多数情况下返回的规范化路径是不包含末尾斜杠的（除非是根目录）。endsWith('/') 检查虽然增加了健壮性，但在处理标准 QUrl 对象时可能是冗余的。不过，考虑到 url 可能来自外部输入或非标准构造，保留此检查有助于防御性编程。

• 代码质量：

  • 可读性：将三元运算符展开为 if-else 结构，逻辑更清晰，特别是增加了对末尾斜杠的判断后，三元运算符会变得嵌套且难以阅读。
  • 字符串拼接：使用了 QString 的 + 操作符，这在 Qt 中是高效的，因为 QString 内部使用了引用计数和写时复制机制。

• 代码性能：

  • 轻微开销：引入了 QString dirPath 临时变量，这涉及一次字符串拷贝。但在现代 CPU 上，这种开销对于路径字符串来说几乎可以忽略不计。
  • 建议：如果对性能极度敏感，可以使用 QStringBuilder（需包含 <QStringBuilder>）或 QString::asprintf，但在 I/O 密集型操作（如文件枚举）中，这点 CPU 开销通常不是瓶颈。

• 代码安全：

  • 路径拼接安全：代码正确处理了路径分隔符，防止了路径拼接错误。
  • 潜在风险：如果 fileName 包含路径遍历字符（如 ../），构建出的 path 可能会指向预期之外的目录。但考虑到这是在遍历器内部使用，fileName 通常来自底层文件系统（如 GLib），风险较低。如果 fileName 来自不可信的用户输入，建议增加校验。

---

2. hasNext 函数修改分析

修改内容：
将 g_file_enumerator_iterate 替换为 g_file_enumerator_next_file，并简化了 URL 构建逻辑。

GFileInfo *nextInfo = g_file_enumerator_next_file(enumerator, d->cancellable, &gerror);
if (nextInfo) {
    // Build URL from the name field (nextInfo may not include a GFile)
    d->nextUrl = d->buildUrl(d->uri, g_file_info_get_name(nextInfo));

    d->dfileInfoNext = DLocalHelper::createFileInfoByUri(d->nextUrl, g_file_info_dup(nextInfo), FILE_DEFAULT_ATTRIBUTES,
                                                         d->enumLinks ? DFileInfo::FileQueryInfoFlags::kTypeNone : DFileInfo::FileQueryInfoFlags::kTypeNoFollowSymlinks);

    g_object_unref(nextInfo);

    if (gerror) {
        d->setErrorFromGError(gerror);
        g_error_free(gerror);
        gerror = nullptr;
    }

    if (!d->checkFilter())
        return this->hasNext();

    return true;
}

// nextInfo == NULL: either finished or an error occurred
if (gerror) {
    d->setErrorFromGError(gerror);
    return true;
}

审查意见：

• 语法逻辑：

  • API 替换合理性：g_file_enumerator_iterate 是一个“多合一”函数，同时返回 GFileInfo 和 GFile。在某些后端（如网络文件系统或特定挂载点），其行为可能不一致。g_file_enumerator_next_file 更专注于获取下一个文件的信息，逻辑更单一，减少了歧义。
  • URL 构建逻辑：旧代码通过 g_file_get_path 或 g_file_get_uri 获取完整路径/URI，然后转换为 QUrl。新代码直接使用 g_file_info_get_name 获取文件名，然后通过 buildUrl 拼接。这在逻辑上是等价的，但依赖于 buildUrl 的正确性（上一节已确认其逻辑正确）。
  • 错误处理：新代码在 nextInfo 有效时检查 gerror。这在逻辑上略显矛盾，因为 nextInfo != NULL 通常意味着操作成功。如果 gerror 在此时非空，可能是非致命错误（如部分属性读取失败）。保留错误处理是好的，但 return true 可能会让调用者忽略错误。
  • 递归调用：return this->hasNext() 保留了旧代码的递归逻辑，用于跳过不符合过滤条件的文件。这在深层目录结构中可能导致堆栈溢出，虽然在实际文件系统中不太可能发生（路径长度限制）。

• 代码质量：

  • 内存管理：
    • 旧代码使用 g_file_enumerator_iterate，返回的 gfileInfo 和 gfile 由调用者管理（需要 g_object_unref）。
    • 新代码使用 g_file_enumerator_next_file，返回的 GFileInfo 需要手动 g_object_unref。代码中正确地调用了 g_object_unref(nextInfo)，避免了内存泄漏。
    • 旧代码中的 g_autoptr(GError) gerror 会自动释放错误对象。新代码中，gerror 是传递给 g_file_enumerator_next_file 的输出参数，如果函数内部分配了内存，调用者需要释放。代码中使用了 g_error_free(gerror)，这是正确的。
  • 代码简洁性：移除了 GFile *gfile 变量，因为不再需要它来获取路径。代码更简洁。

• 代码性能：

  • 系统调用开销：g_file_enumerator_next_file 可能比 g_file_enumerator_iterate 更轻量级，因为它只获取文件信息，而不创建 GFile 对象。这在枚举大量文件时可能有性能提升。
  • 字符串操作：旧代码需要将路径/URI 从 GLib 的字符串格式转换为 Qt 的 QString。新代码只需要转换文件名（通常比完整路径短），然后通过 Qt 的字符串操作拼接路径。这可能略微减少字符串拷贝和转换的开销。

• 代码安全：

  • 空指针解引用：新代码在 nextInfo 有效时调用 g_file_info_get_name(nextInfo)，这是安全的。
  • 错误处理：新代码在 nextInfo == NULL 时检查 gerror，这是正确的。如果发生错误，调用 d->setErrorFromGError(gerror) 并返回 true，让调用者知道发生了错误。
  • 内存泄漏：如前所述，新代码正确地释放了 nextInfo 和 gerror，避免了内存泄漏。

---

3. 综合改进建议

1. buildUrl 函数：

   • 使用 QDir::cleanPath：在拼接路径后，可以使用 QDir::cleanPath(path) 来规范化路径（如移除多余的 / 和 .），进一步确保路径的正确性。
   • 代码风格：可以将 QString(fileName) 提取为变量，避免重复转换。

2. hasNext 函数：

   • 错误处理逻辑：建议明确 gerror 在 nextInfo != NULL 时的含义。如果是非致命错误，可以考虑记录日志而不是设置错误状态。如果是致命错误，应该返回 false 停止枚举。
   • 递归优化：虽然递归深度不太可能成为问题，但可以将递归改为循环，以彻底消除堆栈溢出的风险。

3. 通用建议：

   • 代码注释：代码中已有注释解释了使用 g_file_enumerator_next_file 的原因，这很好。建议在 buildUrl 中也添加注释，说明路径拼接的逻辑。
   • 单元测试：建议增加单元测试，覆盖各种边界情况，如根路径、带斜杠的路径、空文件名等。

4. 总结

这份代码修改在逻辑上是正确的，提高了代码的可读性和健壮性，同时可能带来轻微的性能提升。主要改进点包括：

• 更清晰的路径拼接逻辑：buildUrl 函数现在能正确处理各种路径格式。
• 更单一职责的 API 使用：hasNext 函数使用 g_file_enumerator_next_file，减少了歧义。
• 正确的内存管理：确保了 GLib 对象的正确释放。

建议的改进主要集中在错误处理的明确性和防御性编程的增强上，但这些都不是严重问题。整体而言，这是一份高质量的代码修改。

[pppanghu77]

looks greats

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, liyigang1, max-lvs, pppanghu77

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[GongHeng2017]

/merge