public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm: property: use kzalloc_flex
@ 2026-03-13  0:06 Rosen Penev
  2026-03-13  3:43 ` Claude review: " Claude Code Review Bot
  2026-03-13  3:43 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Rosen Penev @ 2026-03-13  0:06 UTC (permalink / raw)
  To: dri-devel
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Kees Cook, Gustavo A. R. Silva, open list

Don't allocate values separately to simplify allocation.

Add __counted_by for extra runtime analysis. Move the counting variable
assignment to right after allocation as required by __counted_by.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/gpu/drm/drm_property.c | 14 ++------------
 include/drm/drm_property.h     | 16 ++++++++--------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index f38f2c5437e6..a6008e5c2b58 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -107,25 +107,18 @@ struct drm_property *drm_property_create(struct drm_device *dev,
 	if (WARN_ON(strlen(name) >= DRM_PROP_NAME_LEN))
 		return NULL;
 
-	property = kzalloc_obj(struct drm_property);
+	property = kzalloc_flex(*property, values, num_values);
 	if (!property)
 		return NULL;
 
+	property->num_values = num_values;
 	property->dev = dev;
 
-	if (num_values) {
-		property->values = kcalloc(num_values, sizeof(uint64_t),
-					   GFP_KERNEL);
-		if (!property->values)
-			goto fail;
-	}
-
 	ret = drm_mode_object_add(dev, &property->base, DRM_MODE_OBJECT_PROPERTY);
 	if (ret)
 		goto fail;
 
 	property->flags = flags;
-	property->num_values = num_values;
 	INIT_LIST_HEAD(&property->enum_list);
 
 	strscpy_pad(property->name, name, DRM_PROP_NAME_LEN);
@@ -134,7 +127,6 @@ struct drm_property *drm_property_create(struct drm_device *dev,
 
 	return property;
 fail:
-	kfree(property->values);
 	kfree(property);
 	return NULL;
 }
@@ -447,8 +439,6 @@ void drm_property_destroy(struct drm_device *dev, struct drm_property *property)
 		kfree(prop_enum);
 	}
 
-	if (property->num_values)
-		kfree(property->values);
 	drm_mode_object_unregister(dev, &property->base);
 	list_del(&property->head);
 	kfree(property);
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index aa49b5a42bb5..8c7cef4418c4 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -175,14 +175,6 @@ struct drm_property {
 	 */
 	uint32_t num_values;
 
-	/**
-	 * @values:
-	 *
-	 * Array with limits and values for the property. The
-	 * interpretation of these limits is dependent upon the type per @flags.
-	 */
-	uint64_t *values;
-
 	/**
 	 * @dev: DRM device
 	 */
@@ -195,6 +187,14 @@ struct drm_property {
 	 * enum and bitmask values.
 	 */
 	struct list_head enum_list;
+
+	/**
+	 * @values:
+	 *
+	 * Array with limits and values for the property. The
+	 * interpretation of these limits is dependent upon the type per @flags.
+	 */
+	uint64_t values[] __counted_by(num_values);
 };
 
 /**
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm: property: use kzalloc_flex
  2026-03-13  0:06 [PATCH] drm: property: use kzalloc_flex Rosen Penev
@ 2026-03-13  3:43 ` Claude Code Review Bot
  2026-03-13  3:43 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-13  3:43 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm: property: use kzalloc_flex
Author: Rosen Penev <rosenp@gmail.com>
Patches: 1
Reviewed: 2026-03-13T13:43:38.093496

---

This is a single-patch series that converts `struct drm_property` to use a flexible array member for `values[]` instead of a separately allocated pointer, using the `kzalloc_flex` helper. The conversion is straightforward and correct. The `values` pointer was never reassigned after initial allocation, `struct drm_property` is always heap-allocated via pointers, and all existing accesses to `property->values[i]` work identically with a flexible array member.

The patch is **clean and correct**. It simplifies allocation (removing one `kcalloc` and its error handling), simplifies deallocation (removing the separate `kfree(property->values)`), and adds `__counted_by` for runtime bounds checking — all good improvements.

**Recommendation: Accept / Reviewed-by worthy**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm: property: use kzalloc_flex
  2026-03-13  0:06 [PATCH] drm: property: use kzalloc_flex Rosen Penev
  2026-03-13  3:43 ` Claude review: " Claude Code Review Bot
@ 2026-03-13  3:43 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-13  3:43 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good**

- The conversion from a separate `uint64_t *values` pointer to `uint64_t values[] __counted_by(num_values)` flexible array member is valid. The struct is only ever allocated on the heap via `drm_property_create()`, never embedded in another struct or placed on the stack.

- The `kzalloc_flex(*property, values, num_values)` call correctly allocates `sizeof(struct drm_property) + num_values * sizeof(uint64_t)`.

- Moving `property->num_values = num_values` to immediately after allocation is required by `__counted_by` semantics — the count field must be set before any access to the flexible array. This is done correctly.

**Cleanup correctness: Good**

- Removing `kfree(property->values)` from both the error path in `drm_property_create()` and from `drm_property_destroy()` is correct since the values are now part of the same allocation as the struct.

- The `if (property->num_values)` guard before `kfree(property->values)` was unnecessary even before (kfree(NULL) is safe), but removing both lines is the right thing with the flexible array.

**Minor observations:**

- The original code had `if (num_values)` guarding the `kcalloc` — with `kzalloc_flex`, a `num_values` of 0 will allocate just `sizeof(struct drm_property)` with no trailing array space, which is fine. All callers that pass `num_values=0` never access `property->values[]`, so this is safe.

- No other in-tree code assigns to `property->values` as a pointer (verified via grep), so there are no missed conversion sites.

No issues found. The patch is ready to merge.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-13  3:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-13  0:06 [PATCH] drm: property: use kzalloc_flex Rosen Penev
2026-03-13  3:43 ` Claude review: " Claude Code Review Bot
2026-03-13  3:43 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox