From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: of_reserved_mem: add a helper for rmem device_init op Date: Wed, 04 Mar 2026 07:19:20 +1000 Message-ID: In-Reply-To: <20260303-b4-dmabuf-heap-coherent-rmem-v2-3-65a4653b3378@redhat.com> References: <20260303-b4-dmabuf-heap-coherent-rmem-v2-0-65a4653b3378@redhat.com> <20260303-b4-dmabuf-heap-coherent-rmem-v2-3-65a4653b3378@redhat.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Adds `of_reserved_mem_device_init_with_mem()` that calls `rmem->ops->device_init()` directly, and refactors `of_reserved_mem_device_init_by_idx()` to use it. **Issues:** 1. **Memory leak introduced in `of_reserved_mem_device_init_by_idx()`.** The original code checks `if (!rmem || !rmem->ops || !rmem->ops->device_init) return -EINVAL;` **before** the `kmalloc_obj()`. The refactored code moves the validation into `of_reserved_mem_device_init_with_mem()`, but now `rd` is allocated first: ```c - if (!rmem || !rmem->ops || !rmem->ops->device_init) - return -EINVAL; - rd = kmalloc_obj(struct rmem_assigned_device); if (!rd) return -ENOMEM; - ret = rmem->ops->device_init(rmem, dev); + ret = of_reserved_mem_device_init_with_mem(dev, rmem); ``` If `rmem` is NULL or has no ops, the new helper returns `-EINVAL`, but `rd` was already allocated and is only freed in the `else` branch of `if (ret == 0)`. Looking at the original code after the hunk: `if (ret == 0) { ... } else { kfree(rd); }` -- so `rd` does get freed on the error path. This is actually fine. However, there's a subtler issue: the original code never reached `kmalloc_obj()` when `rmem` was NULL, but now it does, wasting memory only to immediately free it. Not a bug per se, but a minor inefficiency change. 2. **Good:** The `#ifndef CONFIG_OF_RESERVED_MEM` stub returns `-EOPNOTSUPP` which is appropriate. --- Generated by Claude Code Patch Reviewer