Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The commit message is factually incorrect regarding the reference counting behavior of `mock_chain()`. It claims that `mock_chain()` acquires its own references, making the `dma_fence_get()` calls on lines 440 and 445 surplus and permanently leaked. In reality, `mock_chain()` (and underlying `dma_fence_chain_init()`) takes ownership of the references passed to it without acquiring new ones. Since the original references to `f1` and `f2` were already consumed by `mock_array()`, the `dma_fence_get()` calls are strictly necessary to pass new references to `mock_chain()` and avoid a double-free. Furthermore, the commit message implies `c1` is leaked, but the patch correctly omits `dma_fence_put(c1)` because ownership of `c1` is transferred to `c2`. --
commit 96e1c6f7abe617d57c291e7a9fc66a1436a69266 Author: WenTao Liang <[email protected]> fix: dma-buf: unwrap_merge_order: success path leaks chain and array fence references This commit adds missing dma_fence_put() calls to fix resource leaks in the unwrap_merge_order() success path. > The success path only releases a2 via dma_fence_put but does not release > a1, c1, or c2. The dma_fence_get calls at lines 440 and 445 were intended > to pass references to mock_chain, but mock_chain already acquires its own > references internally, making these extra gets surplus and permanently > leaked. [Severity: Medium] Is this description accurate regarding how mock_chain() handles reference counting? The commit message states that mock_chain() "already acquires its own references internally". However, mock_chain() and the underlying dma_fence_chain_init() actually take ownership of the reference passed to them without acquiring new ones. Because the original references to f1 and f2 are consumed by mock_array(), the dma_fence_get() calls seem to be necessary to pass new references to mock_chain() and avoid a double-free, rather than being surplus. Additionally, the commit message claims that c1 is leaked, but the patch correctly omits a dma_fence_put(c1) because ownership of c1 is transferred to c2. Could the commit message be updated to avoid implying c1 needs to be released or that the get calls are surplus? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
