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: Fix initial memory allocation for Audio PD memory pool
Date: Sun, 12 Apr 2026 11:44:33 +1000
Message-ID: <review-patch1-20260409062617.1182-2-jianping.li@oss.qualcomm.com>
In-Reply-To: <20260409062617.1182-2-jianping.li@oss.qualcomm.com>
References: <20260409062617.1182-1-jianping.li@oss.qualcomm.com> <20260409062617.1182-2-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

**Verdict: Looks correct.**

The bug analysis is sound. In the original code, `inbuf.pageslen` is unconditionally set to 0 *after* the remote_heap allocation block:

```c
// Original code (after the allocation/SCM block):
inbuf.client_id = fl->client_id;
inbuf.namelen = init.namelen;
inbuf.pageslen = 0;  // always 0, even if heap was just allocated
```

This means the buffer page info is never passed to the DSP, so the memory pool starts empty and the initial allocation is leaked.

The fix moves the `inbuf` initialization before the allocation block and conditionally sets `pageslen = 1` when SCM assignment succeeds:

```c
inbuf.pageslen = 0;
if (!fl->cctx->remote_heap) {
    // allocate + SCM...
    scm_done = true;
    inbuf.pageslen = 1;  // <-- now set when memory is actually registered
}
```

This correctly distinguishes first-time allocation (pageslen=1, send page info) from subsequent calls where the heap already exists (pageslen=0). Note that Patch 3 later removes and replaces this code entirely, but each patch in the series is independently correct.

**Minor nit:** The blank line after `goto err;` followed by `}` and the removed blank line before `if (!fl->cctx->remote_heap)` changes the spacing slightly. No functional issue.

---

---
Generated by Claude Code Patch Reviewer