From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/tegra: Fix iommu_map_sgtable() return value check Date: Thu, 23 Apr 2026 08:57:24 +1000 Message-ID: In-Reply-To: <20260421-iommu_map_sgtable-return-v1-2-fb484c07d2a1@nvidia.com> References: <20260421-iommu_map_sgtable-return-v1-0-fb484c07d2a1@nvidia.com> <20260421-iommu_map_sgtable-return-v1-2-fb484c07d2a1@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 **Status: Good** This patch introduces a local `ssize_t size` to capture the return value, checks `< 0`, and only assigns to `bo->size` on success: ```c + size = iommu_map_sgtable(tegra->domain, bo->iova, bo->sgt, prot); + if (size < 0) { dev_err(tegra->drm->dev, "failed to map buffer\n"); - err = -ENOMEM; + err = size; goto remove; } + + bo->size = size; ``` The deferral of `bo->size = size` to after the error check is a nice improvement -- the old code would write zero into `bo->size` on the error path (since it checked `!bo->size`), which was harmless but sloppy. Now `bo->size` is only set on success. One minor note: `bo->size` is declared as `size_t` in `gem.h:68`. Assigning from `ssize_t` to `size_t` is safe here because it only happens after the `< 0` check, so the value is guaranteed non-negative. This is fine. Similarly, `err = size` narrows `ssize_t` to `int` but only on the error path (negative errno), which is safe. No issues. --- Generated by Claude Code Patch Reviewer