public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/vkms: Constify struct configfs_item_operations and configfs_group_operations
@ 2026-05-25 12:30 Christophe JAILLET
  2026-05-25 21:16 ` Claude review: " Claude Code Review Bot
  2026-05-25 21:16 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Christophe JAILLET @ 2026-05-25 12:30 UTC (permalink / raw)
  To: Louis Chauvet, Haneen Mohammed, Simona Vetter, Melissa Wen,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
  Cc: linux-kernel, kernel-janitors, Christophe JAILLET, dri-devel

'struct configfs_item_operations' and 'configfs_group_operations' are not
modified in this driver.

Constifying these structures moves some data to a read-only section, so
increases overall security, especially when the structure holds some
function pointers.

On a x86_64, with allmodconfig, as an example:
Before:
======
   text	   data	    bss	    dec	    hex	filename
  21554	  10096	    128	  31778	   7c22	drivers/gpu/drm/vkms/vkms_configfs.o

After:
=====
   text	   data	    bss	    dec	    hex	filename
  22546	   9104	    128	  31778	   7c22	drivers/gpu/drm/vkms/vkms_configfs.o

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.
---
 drivers/gpu/drm/vkms/vkms_configfs.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index 7551b8c7766d..b2b3343f9d35 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -189,7 +189,7 @@ static void crtc_release(struct config_item *item)
 	}
 }
 
-static struct configfs_item_operations crtc_item_operations = {
+static const struct configfs_item_operations crtc_item_operations = {
 	.release	= &crtc_release,
 };
 
@@ -231,7 +231,7 @@ static struct config_group *make_crtc_group(struct config_group *group,
 	return &crtc->group;
 }
 
-static struct configfs_group_operations crtcs_group_operations = {
+static const struct configfs_group_operations crtcs_group_operations = {
 	.make_group	= &make_crtc_group,
 };
 
@@ -276,7 +276,7 @@ static void plane_possible_crtcs_drop_link(struct config_item *src,
 		vkms_config_plane_detach_crtc(plane->config, crtc->config);
 }
 
-static struct configfs_item_operations plane_possible_crtcs_item_operations = {
+static const struct configfs_item_operations plane_possible_crtcs_item_operations = {
 	.allow_link	= plane_possible_crtcs_allow_link,
 	.drop_link	= plane_possible_crtcs_drop_link,
 };
@@ -345,7 +345,7 @@ static void plane_release(struct config_item *item)
 	}
 }
 
-static struct configfs_item_operations plane_item_operations = {
+static const struct configfs_item_operations plane_item_operations = {
 	.release	= &plane_release,
 };
 
@@ -393,7 +393,7 @@ static struct config_group *make_plane_group(struct config_group *group,
 	return &plane->group;
 }
 
-static struct configfs_group_operations planes_group_operations = {
+static const struct configfs_group_operations planes_group_operations = {
 	.make_group	= &make_plane_group,
 };
 
@@ -438,7 +438,7 @@ static void encoder_possible_crtcs_drop_link(struct config_item *src,
 		vkms_config_encoder_detach_crtc(encoder->config, crtc->config);
 }
 
-static struct configfs_item_operations encoder_possible_crtcs_item_operations = {
+static const struct configfs_item_operations encoder_possible_crtcs_item_operations = {
 	.allow_link	= encoder_possible_crtcs_allow_link,
 	.drop_link	= encoder_possible_crtcs_drop_link,
 };
@@ -462,7 +462,7 @@ static void encoder_release(struct config_item *item)
 	}
 }
 
-static struct configfs_item_operations encoder_item_operations = {
+static const struct configfs_item_operations encoder_item_operations = {
 	.release	= &encoder_release,
 };
 
@@ -510,7 +510,7 @@ static struct config_group *make_encoder_group(struct config_group *group,
 	return &encoder->group;
 }
 
-static struct configfs_group_operations encoders_group_operations = {
+static const struct configfs_group_operations encoders_group_operations = {
 	.make_group	= &make_encoder_group,
 };
 
@@ -579,7 +579,7 @@ static void connector_release(struct config_item *item)
 	}
 }
 
-static struct configfs_item_operations connector_item_operations = {
+static const struct configfs_item_operations connector_item_operations = {
 	.release	= &connector_release,
 };
 
@@ -628,7 +628,7 @@ static void connector_possible_encoders_drop_link(struct config_item *src,
 	}
 }
 
-static struct configfs_item_operations connector_possible_encoders_item_operations = {
+static const struct configfs_item_operations connector_possible_encoders_item_operations = {
 	.allow_link	= connector_possible_encoders_allow_link,
 	.drop_link	= connector_possible_encoders_drop_link,
 };
@@ -677,7 +677,7 @@ static struct config_group *make_connector_group(struct config_group *group,
 	return &connector->group;
 }
 
-static struct configfs_group_operations connectors_group_operations = {
+static const struct configfs_group_operations connectors_group_operations = {
 	.make_group	= &make_connector_group,
 };
 
@@ -750,7 +750,7 @@ static void device_release(struct config_item *item)
 	kfree(dev);
 }
 
-static struct configfs_item_operations device_item_operations = {
+static const struct configfs_item_operations device_item_operations = {
 	.release	= &device_release,
 };
 
@@ -802,7 +802,7 @@ static struct config_group *make_device_group(struct config_group *group,
 	return &dev->group;
 }
 
-static struct configfs_group_operations device_group_ops = {
+static const struct configfs_group_operations device_group_ops = {
 	.make_group = &make_device_group,
 };
 
-- 
2.54.0


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

* Claude review: drm/vkms: Constify struct configfs_item_operations and configfs_group_operations
  2026-05-25 12:30 [PATCH] drm/vkms: Constify struct configfs_item_operations and configfs_group_operations Christophe JAILLET
  2026-05-25 21:16 ` Claude review: " Claude Code Review Bot
@ 2026-05-25 21:16 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 21:16 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/vkms: Constify struct configfs_item_operations and configfs_group_operations
Author: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Patches: 1
Reviewed: 2026-05-26T07:16:38.236375

---

This is a single-patch cleanup that adds `const` qualifiers to all `struct configfs_item_operations` and `struct configfs_group_operations` instances in the vkms configfs driver. The change is trivially correct, well-motivated, and a net positive for security and code hygiene.

The `config_item_type` structure in `include/linux/configfs.h:67-68` already declares these fields as `const` pointers:

```c
const struct configfs_item_operations    *ct_item_ops;
const struct configfs_group_operations   *ct_group_ops;
```

So the framework explicitly expects and supports `const` here. Other subsystems (e.g., USB gadget configfs) already use `const` for these structures. The commit message is clear, includes before/after size data showing the expected text/data shift (~992 bytes moved from writable data to read-only text), and notes it was compile-tested.

**Verdict: Patch looks good. No issues found.**

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/vkms: Constify struct configfs_item_operations and configfs_group_operations
  2026-05-25 12:30 [PATCH] drm/vkms: Constify struct configfs_item_operations and configfs_group_operations Christophe JAILLET
@ 2026-05-25 21:16 ` Claude Code Review Bot
  2026-05-25 21:16 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 21:16 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Clean**

The patch adds `const` to 13 structure definitions across `drivers/gpu/drm/vkms/vkms_configfs.c`:

- 8 `configfs_item_operations` instances: `crtc_item_operations`, `plane_possible_crtcs_item_operations`, `plane_item_operations`, `encoder_possible_crtcs_item_operations`, `encoder_item_operations`, `connector_item_operations`, `connector_possible_encoders_item_operations`, `device_item_operations`
- 5 `configfs_group_operations` instances: `crtcs_group_operations`, `planes_group_operations`, `encoders_group_operations`, `connectors_group_operations`, `device_group_ops`

All of these are statically initialized with function pointers and never modified after initialization, making `const` appropriate. The parent `config_item_type` struct stores these via `const` pointers, so no cast or type mismatch is introduced.

The size data in the commit message is consistent: `text` increases by 992 bytes, `data` decreases by 992 bytes, and the total (`dec`) stays at 31778, confirming the structures moved from `.data` to `.rodata` as expected.

No correctness, style, or completeness issues.

**Reviewed-by: appropriate.**

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-05-25 21:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-25 12:30 [PATCH] drm/vkms: Constify struct configfs_item_operations and configfs_group_operations Christophe JAILLET
2026-05-25 21:16 ` Claude review: " Claude Code Review Bot
2026-05-25 21:16 ` 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