Re: [PATCH v2] tty: vt/keyboard: Hoist and reuse variable in vt_do_kdgkb_ioctl

[Thorsten Blum]

On 12. Mar 2026, at 15:18, Greg Kroah-Hartman wrote:
> On Mon, Mar 02, 2026 at 04:32:52PM +0100, Thorsten Blum wrote:
>> Hoist 'len' and use it in both cases.
> 
> Why?  And what is "both cases"?

To reuse 'len' in both switch cases (KDGKBSENT and KDSKBSENT) instead of
defining 'len = sizeof(user_kdgkb->kb_string)' in KDGKBSENT and inlining
sizeof(user_kdgkb->kb_string) in KDSKBSENT.

>> Add a comment explaining why reassigning 'kbs' is intentional.
>> 
>> Signed-off-by: Thorsten Blum <thorsten.blum@xxxxxxxxx>
>> ---
>> Changes in v2:
>> - Keep 'kbs' reassignment and add a comment why it's required (Jiri)
>> - Link to v1: https://lore.kernel.org/lkml/20260226123419.737669-1-thorsten.blum@xxxxxxxxx/
>> ---
>> drivers/tty/vt/keyboard.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
> 
> I feel you just made the code harder to understand, as you added
> complexity :(

Not sure how reusing a local variable adds complexity? Would renaming
'len' to 'kb_string_len' help?

>> 		guard(spinlock_irqsave)(&func_buf_lock);
>> +
>> +		/*
>> +		 * Ownership transfer: vt_kdskbsent() returns a pointer
>> +		 * that must be freed (new buffer, old buffer, or NULL).
>> +		 */
>> 		kbs = vt_kdskbsent(kbs, kb_func);
> 
> That's fine, but what does it have to do with len?

It's unrelated to 'len' and just a drive-by change while I was at it.

Thanks,
Thorsten