* [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