From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: gpu: nova-core: gsp: warn if data remains after processing a message Date: Tue, 17 Feb 2026 15:48:06 +1000 Message-ID: In-Reply-To: <20260217-nova-misc-v3-1-b4e2d45eafbc@nvidia.com> References: <20260217-nova-misc-v3-0-b4e2d45eafbc@nvidia.com> <20260217-nova-misc-v3-1-b4e2d45eafbc@nvidia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This patch uses `.inspect()` on the `Result` to check whether the `SBufferIter` still has unprocessed data after `M::read()` completes. The use of `inspect` is appropriate - it only runs the closure on `Ok`, which is the right time to check for leftover data. The `function` variable is in scope (defined at line 656 of the original), so the debug format works correctly. > + M::read(cmd, &mut sbuffer) > + .map_err(|e| e.into()) > + .inspect(|_| { > + if !sbuffer.is_empty() { > + dev_warn!( > + &self.dev, > + "GSP message {:?} has unprocessed data\n", > + function > + ); > + } > + }) One observation: `inspect` runs after `map_err`, so if `M::read` returns an error, we don't warn about unprocessed data. That seems intentional - if the read failed, there's no point warning about leftover bytes. Note that `function` is formatted with `{:?}` (Debug format). The existing code in `send_command` uses `{}` (Display format) for the same type, but patch 2 removes the `Display` impl and changes that to `{:?}` as well - so the series is self-consistent. No issues found. --- Generated by Claude Code Patch Reviewer