From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: misc: fastrpc: Allow fastrpc_buf_free() to accept NULL
Date: Thu, 04 Jun 2026 13:11:16 +1000
Message-ID: <review-patch5-20260602071750.526-6-jianping.li@oss.qualcomm.com>
In-Reply-To: <20260602071750.526-6-jianping.li@oss.qualcomm.com>
References: <20260602071750.526-1-jianping.li@oss.qualcomm.com> <20260602071750.526-6-jianping.li@oss.qualcomm.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Purpose:** Add a NULL guard to `fastrpc_buf_free()` so callers don't need to check.

```c
 static void fastrpc_buf_free(struct fastrpc_buf *buf)
 {
+	if (!buf)
+		return;
+
 	dma_free_coherent(buf->dev, buf->size, buf->virt,
```

And removes two caller-side NULL checks:
```c
-	if (fl->init_mem)
-		fastrpc_buf_free(fl->init_mem);
+	fastrpc_buf_free(fl->init_mem);
```

```c
-	if (ctx->buf)
-		fastrpc_buf_free(ctx->buf);
+	fastrpc_buf_free(ctx->buf);
```

**Note:** The patch applies to a function `fastrpc_user_free` that doesn't exist in the current upstream tree. It's likely that the line numbers correspond to `fastrpc_device_release` (line 1593 upstream). Since the patches don't apply cleanly, this may be based on a slightly different tree. The change itself is correct regardless.

**Verdict:** Clean, straightforward cleanup. This is a common pattern in kernel code (cf. `kfree(NULL)`). No issues.

---

**Summary of actionable items:**
1. **Patch 3 & 4 ordering:** Patch 3's validation makes no sense without patch 4. Swap their order or squash them.
2. **Patch 3:** Use `-ENODEV` or `-EINVAL` instead of `-ENOMEM`.
3. **Patch 4 probe:** `return -ENOMEM` should be `goto err_free_data` to avoid leaking earlier allocations.
4. **Patch 4 probe:** `remote_heap` is leaked if `qcom_scm_assign_mem` fails.
5. **Patch 4:** Verify SDSP is not regressed by the removal of lazy `remote_heap` allocation in `fastrpc_init_create_static_process`.
6. **Patch 4:** Use `cctx->` consistently instead of mixing with `fl->cctx->`.

---
Generated by Claude Code Patch Reviewer