fix(dxinput): fix property reading and button mapping data processing logic

[electricface]

• Replaced custom uchar array conversion function with C.GoBytes
  to simplify data copying
• Modified get_prop function implementation to first obtain the
  attribute data size, then read the data completely
• Corrected get_prop function parameters and comments; nbytes now
  indicates byte length rather than item count
• Added free_prop_data function to release memory returned by
  get_prop, preventing memory leaks
• Ensured proper release of C-allocated data after calling get_prop
  to retrieve properties in the Go layer
• Removed unused maxBufferLen constant and the old ucharArrayToByte
  implementation

PMS: BUG-315115
Influence: Input Device Management

---

fix(dxinput): 修正属性读取和按钮映射数据处理逻辑

• 使用 C.GoBytes 替代自定义 uchar
  数组转换函数简化数据拷贝
• 修改 get_prop
  函数实现，先获取属性数据大小，再完整读取数据
• 修正 get_prop 函数参数和注释，nbytes
  表示字节长度而非项目数
• 添加 free_prop_data 函数用于释放 get_prop
  返回的内存，避免内存泄漏
• 在 Go 层调用 get_prop 获取属性后正确释放 C
  分配的数据
• 删除无用的 maxBufferLen 常量和 ucharArrayToByte 旧实现

Influence: 输入设备管理功能

Summary by Sourcery

Improve XInput property retrieval and button mapping data handling to use correct byte lengths and proper memory management between C and Go layers.

Bug Fixes:

• Fix XInput property reading to query the actual property size and read the complete data instead of relying on a fixed buffer length.
• Correct the interpretation of returned property size so that nbytes reflects the true byte length of the data passed to Go.
• Ensure button mapping retrieval uses the actual returned length when converting C arrays to Go byte slices.

Enhancements:

• Replace custom uchar-to-byte conversion helpers with C.GoBytes for simpler and safer data copying between C and Go.
• Introduce a dedicated free_prop_data helper to centralize freeing of XInput property data from Go code.
• Remove obsolete buffer size constants and unused conversion helpers from the dxinput utilities.

[sourcery-ai]

Reviewer's Guide

Refactors XInput property and button mapping handling to correctly size, copy, and free C-allocated buffers, switching to length-aware APIs and GoBytes, and adding a dedicated C-side free helper to prevent leaks.

Sequence diagram for updated GetProperty data and memory handling

sequenceDiagram
    participant GoWrapper as Go_GetProperty
    participant CLayer as C_get_prop
    participant X11 as XInput_XIGetProperty
    participant CFree as C_free_prop_data

    GoWrapper->>CLayer: get_prop(id, prop, &nbytes)
    activate CLayer
    CLayer->>X11: XIGetProperty(length=0)
    X11-->>CLayer: act_type, act_format, num_items, bytes_after, data
    CLayer->>CLayer: check ret and act_type
    CLayer->>CLayer: XFree(data) if not null
    CLayer->>CLayer: if bytes_after == 0 return NULL
    CLayer->>X11: XIGetProperty(length=(bytes_after+3)/4)
    X11-->>CLayer: act_type, act_format, num_items, bytes_after, data
    CLayer->>CLayer: compute nbytes = num_items * (act_format/8)
    CLayer-->>GoWrapper: data pointer, nbytes
    deactivate CLayer

    GoWrapper->>GoWrapper: datas = C.GoBytes(data, nbytes)
    GoWrapper->>CFree: free_prop_data(data)
    CFree-->>GoWrapper: 
    GoWrapper-->>GoWrapper: return datas, int32(nbytes)

Loading

Sequence diagram for updated GetButtonMap data copying

sequenceDiagram
    participant GoWrapper as Go_GetButtonMap
    participant CLayer as C_get_button_map

    GoWrapper->>CLayer: get_button_map(xid, devName, &cbtnNum)
    CLayer-->>GoWrapper: cbtnMap pointer, cbtnNum
    GoWrapper->>GoWrapper: btnMap = C.GoBytes(cbtnMap, cbtnNum)
    GoWrapper->>CLayer: C.free(cbtnMap)
    CLayer-->>GoWrapper: 
    GoWrapper-->>GoWrapper: return btnMap, nil

Loading

File-Level Changes

Change	Details	Files
Make XInput property retrieval length-aware and safer on the C side.	Change get_prop signature/semantics so the out-parameter is a byte-length (nbytes) instead of an item count. Reimplement get_prop to first query the property size with XIGetProperty(length=0), then re-read with an exact length in 32-bit units. Compute the actual byte length from num_items and act_format and store it in nbytes, handling empty properties gracefully. Add free_prop_data wrapper around XFree to free buffers returned by get_prop from Go code.	dxinput/utils/property.c dxinput/utils/property.h
Update Go wrapper to use C.GoBytes and correctly handle C-allocated memory for properties.	Adjust GetProperty to pass a byte-length pointer to get_prop and treat the result as a byte count. Replace custom ucharArrayToByte/maxBufferLen logic with C.GoBytes using the exact nbytes from C. Ensure C strings and property buffers are freed via defer (C.free for property names, free_prop_data for XIGetProperty data). Return the byte slice and byte length from GetProperty instead of item counts.	dxinput/utils/wrapper.go
Simplify button mapping data copying in Go.	Replace ucharArrayToByte usage in GetButtonMap with C.GoBytes sized by the C-reported button count. Rely on the XInput API’s reported size instead of a fixed buffer length helper.	dxinput/utils/button_map.go

---

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In get_prop, nbytes is computed as num_items * (act_format / 8) without validating that act_format is one of the expected 8/16/32 values or that the multiplication fits into an int; consider adding a format sanity check and range check to avoid silent truncation or zero-length results for unexpected formats.
• The Go GetProperty API now returns a byte-length as the second value instead of an item count; if callers rely on an element count, it may be worth clarifying the unit in the function name or adding a small helper to convert to item counts based on the expected element size at call sites.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `get_prop`, `nbytes` is computed as `num_items * (act_format / 8)` without validating that `act_format` is one of the expected 8/16/32 values or that the multiplication fits into an `int`; consider adding a format sanity check and range check to avoid silent truncation or zero-length results for unexpected formats.
- The Go `GetProperty` API now returns a byte-length as the second value instead of an item count; if callers rely on an element count, it may be worth clarifying the unit in the function name or adding a small helper to convert to item counts based on the expected element size at call sites.

## Individual Comments

### Comment 1
<location> `dxinput/utils/property.c:103-105` </location>
<code_context>

-    *nitems = (int)num_items;
-    XCloseDisplay(disp);
+    // Calculate actual byte length based on format and num_items
+    // format is 8, 16, or 32 bits per item
+    *nbytes = (int)(num_items * (act_format / 8));

+    XCloseDisplay(disp);
</code_context>

<issue_to_address>
**issue (bug_risk):** Guard against integer truncation when converting num_items to int for nbytes.

`num_items` is an `unsigned long` but `*nbytes` is an `int`. On 32‑bit builds, `num_items * (act_format / 8)` can overflow or be truncated when cast to `int`, so Go may call `GoBytes` with a length smaller than the actual allocation. Please either:

- Validate `num_items <= INT_MAX / (act_format / 8)` and fail if it would overflow, or
- Use a wider type for the size (e.g., `long`/`size_t`) and update the Go side.

This avoids subtle memory corruption in large or pathological cases.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[deepin-ci-robot]

deepin pr auto review

这份代码修改主要涉及 X11 输入设备属性的获取与处理，包括 C 语言底层实现和 Go 语言封装层。以下是对代码的审查意见：

1. 语法逻辑

优点：

• C 代码 (property.c)：get_prop 函数重构后逻辑更严谨。采用了两步获取策略（先查询大小，再获取数据），避免了固定缓冲区大小限制，能够正确处理任意长度的属性数据。
• Go 代码 (utils.go)：getInt8Prop, getInt16Prop, getInt32Prop, getFloat32Prop 现在基于字节数 (nBytes) 进行校验，逻辑更加准确。例如，对于 int16，检查字节数是否能被 2 整除，以及字节数除以 2 是否等于预期的项目数。这比之前直接比较 num != nitems 更能反映底层数据的实际布局。
• Go 代码 (wrapper.go)：移除了 ucharArrayToByte 函数，改用标准库的 C.GoBytes，减少了手动内存操作的代码，降低了出错风险。

改进建议：

• C 代码 (property.c)：在 get_prop 中，XCloseDisplay(disp) 的调用位置在成功路径和失败路径中重复出现。虽然在当前的 pthread_mutex 锁定范围内是必须的，但可以考虑使用 defer 风格的清理（C 语言中可用 goto cleanup 模式）来集中处理资源释放，减少代码冗余和遗漏 XCloseDisplay 的风险。

  // 示例：使用 goto cleanup 模式
  unsigned char* get_prop(...) {
      // ... 初始化 ...
      pthread_mutex_lock(&x11_global_mutex);
      // ... 获取 display ...
      
      // Step 1
      ret = XIGetProperty(...);
      if (ret != Success || ...) {
          goto cleanup; // 跳转到清理
      }
      // ... Step 2 ...
      
  cleanup:
      if (data) XFree(data);
      if (disp) XCloseDisplay(disp);
      pthread_mutex_unlock(&x11_global_mutex);
      return NULL;
  }

• Go 代码 (wacom.go)：doAction 函数中 return errors.New(string(out)) 虽然语法正确，但如果 out 包含换行符或大量文本，作为错误信息可能不够友好。建议对 out 进行简单的 strings.TrimSpace 处理。

  if err != nil {
      return errors.New(strings.TrimSpace(string(out)))
  }

2. 代码质量

优点：

• 错误处理：Go 层增加了详细的错误信息，例如 "property data is empty" 或 "byte count ... is not divisible by ..."，极大地提高了调试效率。
• 资源管理：在 wrapper.go 的 GetProperty 中，引入了 defer C.free_prop_data(cdatas)，确保 C 层分配的内存（由 XIGetProperty 返回）能够被正确释放，防止内存泄漏。同时 button_map.go 中也改用了 C.GoBytes，配合 defer C.free，资源管理更清晰。
• 注释：C 代码中增加了注释说明了 get_prop 的返回值类型和调用者责任（需调用 XFree），Go 代码中也增加了关于字节大小计算的注释，提高了可读性。

改进建议：

• C 代码 (property.c)：free_prop_data 是一个很好的封装，但建议在头文件 (property.h) 的注释中明确说明它是 XFree 的包装器，以便其他开发者理解其行为。
• Go 代码 (utils.go)：getInt8Prop 等函数中的校验逻辑有重复模式（检查空、检查整除、检查数量）。可以考虑封装一个通用的辅助函数来处理这些检查，减少重复代码。

  func checkPropBytes(id int32, prop string, datas []byte, nBytes int32, itemSize int32) error {
      if len(datas) == 0 {
          return fmt.Errorf("Get prop '%v -- %s' failed: property data is empty", id, prop)
      }
      if nBytes%itemSize != 0 {
          return fmt.Errorf("Get prop '%v -- %s' failed: byte count %d is not divisible by %d", 
              id, prop, nBytes, itemSize)
      }
      if nBytes/itemSize != nBytes/itemSize { // 这里逻辑需根据实际调用调整，仅作示例
          // ...
      }
      return nil
  }

3. 代码性能

优点：

• C 代码 (property.c)：去掉了 MAX_BUF_LEN 限制，不再需要预先分配大块内存或担心数据截断。虽然引入了两次 X11 调用，但对于大多数属性操作，性能影响可以忽略不计，且换取了正确性和安全性。
• Go 代码 (wrapper.go)：使用 C.GoBytes 替代了手动循环拷贝 ucharArrayToByte。C.GoBytes 是运行时提供的优化函数，通常比手动 Go 循环遍历 C 数组性能更好。

改进建议：

• Go 代码 (wrapper.go)：在 GetProperty 中，C.GoBytes 会进行一次内存分配和拷贝。如果上层调用者（如 getInt8Prop）只是读取数据而不需要修改，可以考虑返回一个切片，该切片底层引用 C 内存（通过 unsafe.Pointer），但这需要极其小心地管理生命周期（必须在 C 内存被 XFree 之前使用完）。鉴于当前代码已经使用了 defer 释放 C 内存，保持现状（拷贝数据）是更安全且性能足够的选择。

4. 代码安全

优点：

• C 代码 (property.c)：
  • 增加了对 nbytes 指针的空指针检查。
  • 增加了对 bytes_after 的检查，处理了属性存在但无数据的情况。
  • 关键改进：添加了整数溢出检查 if (nbytes_ul > INT_MAX)，防止 unsigned long 类型的字节数在转换为 int 时发生截断或溢出，这是非常重要的安全加固。
  • 增加了 free_prop_data 函数，封装了 XFree，为 Go 层提供了安全的内存释放接口。
• Go 代码 (wrapper.go)：
  • 使用 defer 确保 C.free_prop_data 被调用，即使发生 panic 也能释放 C 层内存，防止内存泄漏。
  • GetProperty 返回的字节数 nbytes 现在直接来自 C 层计算，而不是固定的 maxBufferLen，避免了越界读取风险。
• Go 代码 (utils.go)：
  • 严格的类型和长度检查（如 nBytes%2 != 0）防止了读取格式错误的数据导致 panic。

改进建议：

• C 代码 (property.c)：在 get_prop 中，act_format 来自 X11 服务器，理论上应该是 8, 16, 或 32。但为了防御性编程，可以增加对 act_format 的检查，如果它不是这三个值之一，应视为错误并清理资源。

  if (act_format != 8 && act_format != 16 && act_format != 32) {
      fprintf(stderr, "[get_prop] Unsupported format %d for %s\n", act_format, prop);
      goto cleanup;
  }

• Go 代码 (wrapper.go)：C.GoBytes 接受的长度是 C.int。虽然 C 层已经检查了 nbytes <= INT_MAX，但在 Go 侧再次断言一下 nbytes 的范围也无妨，增加一层防御。

  if nbytes < 0 || int(nbytes) > 1<<31-1 { // C.int 通常是 32 位
      return nil, 0
  }

总结

这次代码修改在安全性和健壮性方面有显著提升，特别是 C 层对内存管理和整数溢出的处理，以及 Go 层对数据校验的加强。性能方面也有小幅优化（使用 C.GoBytes）。主要改进点集中在去除硬编码限制、增加错误细节和完善资源释放上。

建议采纳上述关于 C 代码 goto cleanup 模式、act_format 检查以及 Go 代码错误信息格式化的建议，以进一步提升代码的健壮性和可维护性。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface, fly602

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