* [PATCH] fbcon: Declare struct fb_info.fbcon_par as of type struct fbcon_par
@ 2026-02-10 9:43 Thomas Zimmermann
2026-02-10 10:01 ` Helge Deller
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Thomas Zimmermann @ 2026-02-10 9:43 UTC (permalink / raw)
To: deller; +Cc: linux-fbdev, dri-devel, Thomas Zimmermann
The only correct type for the field fbcon_par in struct fb_info
is struct fbcon_par. Declare is as such. The field is a pointer
to fbcon-private data.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
include/linux/fb.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 65fb70382675..b8b6f54f3312 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -18,6 +18,7 @@ struct backlight_device;
struct device;
struct device_node;
struct fb_info;
+struct fbcon_par;
struct file;
struct i2c_adapter;
struct inode;
@@ -506,7 +507,7 @@ struct fb_info {
#define FBINFO_STATE_RUNNING 0
#define FBINFO_STATE_SUSPENDED 1
u32 state; /* Hardware state i.e suspend */
- void *fbcon_par; /* fbcon use-only private area */
+ struct fbcon_par *fbcon_par; /* fbcon use-only private area */
/* From here on everything is device dependent */
void *par;
--
2.52.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fbcon: Declare struct fb_info.fbcon_par as of type struct fbcon_par
2026-02-10 9:43 [PATCH] fbcon: Declare struct fb_info.fbcon_par as of type struct fbcon_par Thomas Zimmermann
@ 2026-02-10 10:01 ` Helge Deller
2026-02-11 6:28 ` Claude review: " Claude Code Review Bot
2026-02-11 6:28 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Helge Deller @ 2026-02-10 10:01 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: linux-fbdev, dri-devel
On 2/10/26 10:43, Thomas Zimmermann wrote:
> The only correct type for the field fbcon_par in struct fb_info
> is struct fbcon_par. Declare is as such. The field is a pointer
> to fbcon-private data.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> include/linux/fb.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
applied.
Thanks!
Helge
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: fbcon: Declare struct fb_info.fbcon_par as of type struct fbcon_par
2026-02-10 9:43 [PATCH] fbcon: Declare struct fb_info.fbcon_par as of type struct fbcon_par Thomas Zimmermann
2026-02-10 10:01 ` Helge Deller
@ 2026-02-11 6:28 ` Claude Code Review Bot
2026-02-11 6:28 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-02-11 6:28 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: fbcon: Declare struct fb_info.fbcon_par as of type struct
fbcon_par
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 2
Reviewed: 2026-02-11T16:28:47.401692
---
This is a single-patch series that improves type safety in the framebuffer console (fbcon) subsystem. The change is straightforward and beneficial - it converts the `fbcon_par` field in `struct fb_info` from a generic `void *` pointer to a properly typed `struct fbcon_par *` pointer.
**Strengths:**
- Improves type safety by replacing void pointer with specific struct pointer
- Zero functional change - purely a type improvement
- Minimal and focused scope
- Already applied by maintainer
**Overall Assessment:** This is a clean, simple type safety improvement that follows kernel best practices. The patch has been reviewed and applied by the maintainer (Helge Deller).
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: fbcon: Declare struct fb_info.fbcon_par as of type struct fbcon_par
2026-02-10 9:43 [PATCH] fbcon: Declare struct fb_info.fbcon_par as of type struct fbcon_par Thomas Zimmermann
2026-02-10 10:01 ` Helge Deller
2026-02-11 6:28 ` Claude review: " Claude Code Review Bot
@ 2026-02-11 6:28 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-02-11 6:28 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Commit Message:**
```
The only correct type for the field fbcon_par in struct fb_info
is struct fbcon_par. Declare is as such. The field is a pointer
to fbcon-private data.
```
The commit message is clear and concise. It explains the motivation (correct typing) and what the field represents. Minor typo: "Declare is as such" should be "Declare it as such", but this doesn't affect the technical quality.
**Code Changes:**
```c
+struct fbcon_par;
```
Good - adds forward declaration to allow the header to reference the struct without requiring the full definition. This is the standard kernel pattern for opaque pointers.
```c
- void *fbcon_par; /* fbcon use-only private area */
+ struct fbcon_par *fbcon_par; /* fbcon use-only private area */
```
**Analysis:**
1. **Type Safety:** Converting from `void *` to `struct fbcon_par *` provides compile-time type checking. This prevents accidental assignment of incorrect pointer types and catches errors at compile time rather than runtime.
2. **API Clarity:** Makes it immediately clear what type of data this field should point to, improving code documentation and maintainability.
3. **Backward Compatibility:** Since `struct fbcon_par *` can be implicitly converted to/from `void *` in C, this change should not break any existing code. However, it will catch type mismatches going forward.
4. **Forward Declaration Pattern:** The use of `struct fbcon_par;` forward declaration is correct and follows kernel style. This allows the header to be self-contained while keeping `struct fbcon_par` as an opaque type from the perspective of fb.h users.
5. **Scope:** The change is limited to the header file definition. All actual users of this field should continue to work unchanged since they already expect `struct fbcon_par *`.
**Potential Concerns:**
1. **Incomplete Review:** Without seeing all the code that accesses `fb_info->fbcon_par`, there's a theoretical risk that some code incorrectly uses this field for other purposes. However, given the field name and comment, this is unlikely, and such misuse would now be caught at compile time.
2. **Build Testing:** The patch should be build-tested across all fbcon-related configurations to ensure no hidden type mismatches exist. The maintainer's acceptance suggests this was done.
**Technical Review:**
- **Correctness:** ✓ Correct approach to type safety improvement
- **Style:** ✓ Follows kernel coding style
- **Maintainability:** ✓ Improves code clarity
- **Performance:** ✓ No runtime impact (zero-cost abstraction)
- **Safety:** ✓ Enhances type safety
**Verdict:** This is a good cleanup patch. The change improves type safety with no functional impact. The forward declaration is properly added, and the change follows kernel conventions.
**Reviewed-by equivalent:** Would provide `Reviewed-by` tag if formally reviewing.
**Note:** The patch has already been applied by maintainer Helge Deller as indicated in the mbox response.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-02-11 6:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-10 9:43 [PATCH] fbcon: Declare struct fb_info.fbcon_par as of type struct fbcon_par Thomas Zimmermann
2026-02-10 10:01 ` Helge Deller
2026-02-11 6:28 ` Claude review: " Claude Code Review Bot
2026-02-11 6:28 ` 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