[PATCH v2 0/2] vt: bracketed paste and cursor position

[PATCH v2 0/2] vt: bracketed paste and cursor position

[Nicolas Pitre]

A different kind of VT console update this time. These patches:

- add bracketed paste support to the VT console

- overcome a /dev/vcsa limitation with cursor position retrieval

In v1 those were submitted together to avoid merge conflicts. Those conflicts
no longer exist but here they are together again for coherence sake.

Changes from v1 (https://lore.kernel.org/all/20250514015554.19978-1-nico@fluxnic.net):

- Changed TIOCL_GETCURSORPOS to VT_GETCONSIZECSRPOS with proper structure.
  Moved to the VT_ space to benefit from unambigous pointer argument and
  vt_compat_ioctl() wrapper. Also motivated by the fact that usage require
  both display size and cursor position so those are joined in one syscall
  requiring a structure.

- Code simplifications suggested by Jiri.

diffstat:
 drivers/tty/vt/selection.c     | 31 +++++++++++++++++++++++++++----
 drivers/tty/vt/vt.c            | 15 +++++++++++++++
 drivers/tty/vt/vt_ioctl.c      | 16 ++++++++++++++++
 include/linux/console_struct.h |  1 +
 include/uapi/linux/tiocl.h     |  1 +
 include/uapi/linux/vt.h        |  9 +++++++++
 6 files changed, 69 insertions(+), 4 deletions(-)

[PATCH v2 1/2] vt: bracketed paste support

[Nicolas Pitre]

From: Nicolas Pitre <npitre@baylibre.com>

This is comprised of 3 aspects:

- Take note of when applications advertise bracketed paste support via
  "\e[?2004h" and "\e[?2004l".

- Insert bracketed paste markers ("\e[200~" and "\e[201~") around pasted
  content in paste_selection() when bracketed paste is active.

- Add TIOCL_GETBRACKETEDPASTE to return bracketed paste status so user
  space daemons implementing cut-and-paste functionality (e.g. gpm,
  BRLTTY) may know when to insert bracketed paste markers.

Link: https://en.wikipedia.org/wiki/Bracketed-paste

Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
---
 drivers/tty/vt/selection.c     | 31 +++++++++++++++++++++++++++----
 drivers/tty/vt/vt.c            | 15 +++++++++++++++
 include/linux/console_struct.h |  1 +
 include/uapi/linux/tiocl.h     |  1 +
 4 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 791e2f1f7c0b..24b0a53e5a79 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -403,6 +403,12 @@ int paste_selection(struct tty_struct *tty)
 	DECLARE_WAITQUEUE(wait, current);
 	int ret = 0;
 
+	bool bp = vc->vc_bracketed_paste;
+	static const char bracketed_paste_start[] = "\033[200~";
+	static const char bracketed_paste_end[]   = "\033[201~";
+	const char *bps = bp ? bracketed_paste_start : NULL;
+	const char *bpe = bp ? bracketed_paste_end : NULL;
+
 	console_lock();
 	poke_blanked_console();
 	console_unlock();
@@ -414,7 +420,7 @@ int paste_selection(struct tty_struct *tty)
 
 	add_wait_queue(&vc->paste_wait, &wait);
 	mutex_lock(&vc_sel.lock);
-	while (vc_sel.buffer && vc_sel.buf_len > pasted) {
+	while (vc_sel.buffer && (vc_sel.buf_len > pasted || bpe)) {
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (signal_pending(current)) {
 			ret = -EINTR;
@@ -427,10 +433,27 @@ int paste_selection(struct tty_struct *tty)
 			continue;
 		}
 		__set_current_state(TASK_RUNNING);
+
+		if (bps) {
+			bps += tty_ldisc_receive_buf(ld, bps, NULL, strlen(bps));
+			if (*bps != '\0')
+				continue;
+			bps = NULL;
+		}
+
 		count = vc_sel.buf_len - pasted;
-		count = tty_ldisc_receive_buf(ld, vc_sel.buffer + pasted, NULL,
-					      count);
-		pasted += count;
+		if (count) {
+			pasted += tty_ldisc_receive_buf(ld, vc_sel.buffer + pasted,
+							NULL, count);
+			if (vc_sel.buf_len > pasted)
+				continue;
+		}
+
+		if (bpe) {
+			bpe += tty_ldisc_receive_buf(ld, bpe, NULL, strlen(bpe));
+			if (*bpe == '\0')
+				bpe = NULL;
+		}
 	}
 	mutex_unlock(&vc_sel.lock);
 	remove_wait_queue(&vc->paste_wait, &wait);
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index efb761454166..ed39d9cb4432 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1870,6 +1870,14 @@ int mouse_reporting(void)
 	return vc_cons[fg_console].d->vc_report_mouse;
 }
 
+/* invoked via ioctl(TIOCLINUX) */
+static int get_bracketed_paste(struct tty_struct *tty)
+{
+	struct vc_data *vc = tty->driver_data;
+
+	return vc->vc_bracketed_paste;
+}
+
 enum {
 	CSI_DEC_hl_CURSOR_KEYS	= 1,	/* CKM: cursor keys send ^[Ox/^[[x */
 	CSI_DEC_hl_132_COLUMNS	= 3,	/* COLM: 80/132 mode switch */
@@ -1880,6 +1888,7 @@ enum {
 	CSI_DEC_hl_MOUSE_X10	= 9,
 	CSI_DEC_hl_SHOW_CURSOR	= 25,	/* TCEM */
 	CSI_DEC_hl_MOUSE_VT200	= 1000,
+	CSI_DEC_hl_BRACKETED_PASTE = 2004,
 };
 
 /* console_lock is held */
@@ -1932,6 +1941,9 @@ static void csi_DEC_hl(struct vc_data *vc, bool on_off)
 		case CSI_DEC_hl_MOUSE_VT200:
 			vc->vc_report_mouse = on_off ? 2 : 0;
 			break;
+		case CSI_DEC_hl_BRACKETED_PASTE:
+			vc->vc_bracketed_paste = on_off;
+			break;
 		}
 }
 
@@ -2157,6 +2169,7 @@ static void reset_terminal(struct vc_data *vc, int do_clear)
 	vc->state.charset	= 0;
 	vc->vc_need_wrap	= 0;
 	vc->vc_report_mouse	= 0;
+	vc->vc_bracketed_paste	= 0;
 	vc->vc_utf              = default_utf8;
 	vc->vc_utf_count	= 0;
 
@@ -3483,6 +3496,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
 		break;
 	case TIOCL_BLANKEDSCREEN:
 		return console_blanked;
+	case TIOCL_GETBRACKETEDPASTE:
+		return get_bracketed_paste(tty);
 	default:
 		return -EINVAL;
 	}
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index 20f564e98552..59b4fec5f254 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -145,6 +145,7 @@ struct vc_data {
 	unsigned int	vc_need_wrap	: 1;
 	unsigned int	vc_can_do_color	: 1;
 	unsigned int	vc_report_mouse : 2;
+	unsigned int	vc_bracketed_paste : 1;
 	unsigned char	vc_utf		: 1;	/* Unicode UTF-8 encoding */
 	unsigned char	vc_utf_count;
 		 int	vc_utf_char;
diff --git a/include/uapi/linux/tiocl.h b/include/uapi/linux/tiocl.h
index b32acc229024..88faba506c3d 100644
--- a/include/uapi/linux/tiocl.h
+++ b/include/uapi/linux/tiocl.h
@@ -36,5 +36,6 @@ struct tiocl_selection {
 #define TIOCL_BLANKSCREEN	14	/* keep screen blank even if a key is pressed */
 #define TIOCL_BLANKEDSCREEN	15	/* return which vt was blanked */
 #define TIOCL_GETKMSGREDIRECT	17	/* get the vt the kernel messages are restricted to */
+#define TIOCL_GETBRACKETEDPASTE	18	/* get whether paste may be bracketed */
 
 #endif /* _LINUX_TIOCL_H */
-- 
2.49.0

[PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console size and cursor position

[Nicolas Pitre]

From: Nicolas Pitre <npitre@baylibre.com>

The console dimension and cursor position are available through the
/dev/vcsa interface already. However the /dev/vcsa header format uses
single-byte fields therefore those values are clamped to 255.

As surprizing as this may seem, some people do use 240-column 67-row
screens (a 1920x1080 monitor with 8x16 pixel fonts) which is getting
close to the limit. Monitors with higher resolution are not uncommon
these days (3840x2160 producing a 480x135 character display) and it is
just a matter of time before someone with, say, a braille display using
the Linux VT console and BRLTTY on such a screen reports a bug about
missing and oddly misaligned screen content.

Let's add VT_GETCONSIZECSRPOS for the retrieval of console size and cursor
position without byte-sized limitations. The actual console size limit as
encoded in vt.c is 32767x32767 so using a short here is appropriate. Then
this can be used to get the cursor position when /dev/vcsa reports 255.

The screen dimension may already be obtained using TIOCGWINSZ and adding
the same information to VT_GETCONSIZECSRPOS might be redundant. However
applications that care about cursor position also care about display
size and having 2 separate system calls to obtain them separately is
wasteful. Also, the cursor position can be queried by writing "\e[6n" to
a tty and reading back the result but that may be done only by the actual
application using that tty and not a sideline observer.

Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
---
 drivers/tty/vt/vt_ioctl.c | 16 ++++++++++++++++
 include/uapi/linux/vt.h   |  9 +++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 4b91072f3a4e..83a3d49535e5 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -951,6 +951,22 @@ int vt_ioctl(struct tty_struct *tty,
 					(unsigned short __user *)arg);
 	case VT_WAITEVENT:
 		return vt_event_wait_ioctl((struct vt_event __user *)arg);
+
+	case VT_GETCONSIZECSRPOS:
+	{
+		struct vt_consizecsrpos concsr;
+
+		console_lock();
+		concsr.con_cols = vc->vc_cols;
+		concsr.con_rows = vc->vc_rows;
+		concsr.csr_col = vc->state.x;
+		concsr.csr_row = vc->state.y;
+		console_unlock();
+		if (copy_to_user(up, &concsr, sizeof(concsr)))
+			return -EFAULT;
+		return 0;
+	}
+
 	default:
 		return -ENOIOCTLCMD;
 	}
diff --git a/include/uapi/linux/vt.h b/include/uapi/linux/vt.h
index e9d39c48520a..e93c8910133b 100644
--- a/include/uapi/linux/vt.h
+++ b/include/uapi/linux/vt.h
@@ -84,4 +84,13 @@ struct vt_setactivate {
 
 #define VT_SETACTIVATE	0x560F	/* Activate and set the mode of a console */
 
+struct vt_consizecsrpos {
+	unsigned short con_rows;	/* number of console rows */
+	unsigned short con_cols;	/* number of console columns */
+	unsigned short csr_row;		/* current cursor's row */
+	unsigned short csr_col;		/* current cursor's column */
+};
+
+#define VT_GETCONSIZECSRPOS 0x5610  /* get console size and cursor position */
+
 #endif /* _UAPI_LINUX_VT_H */
-- 
2.49.0

Re: [PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console size and cursor position

[Jiri Slaby]

On 14. 05. 25, 21:42, Nicolas Pitre wrote:
> From: Nicolas Pitre <npitre@baylibre.com>
> 
> The console dimension and cursor position are available through the
> /dev/vcsa interface already. However the /dev/vcsa header format uses
> single-byte fields therefore those values are clamped to 255.
> 
> As surprizing as this may seem, some people do use 240-column 67-row
> screens (a 1920x1080 monitor with 8x16 pixel fonts) which is getting
> close to the limit. Monitors with higher resolution are not uncommon
> these days (3840x2160 producing a 480x135 character display) and it is
> just a matter of time before someone with, say, a braille display using
> the Linux VT console and BRLTTY on such a screen reports a bug about
> missing and oddly misaligned screen content.
> 
> Let's add VT_GETCONSIZECSRPOS for the retrieval of console size and cursor
> position without byte-sized limitations. The actual console size limit as
> encoded in vt.c is 32767x32767 so using a short here is appropriate. Then
> this can be used to get the cursor position when /dev/vcsa reports 255.
> 
> The screen dimension may already be obtained using TIOCGWINSZ and adding
> the same information to VT_GETCONSIZECSRPOS might be redundant. However
> applications that care about cursor position also care about display
> size and having 2 separate system calls to obtain them separately is
> wasteful. Also, the cursor position can be queried by writing "\e[6n" to
> a tty and reading back the result but that may be done only by the actual
> application using that tty and not a sideline observer.
> 
> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> ---
>   drivers/tty/vt/vt_ioctl.c | 16 ++++++++++++++++
>   include/uapi/linux/vt.h   |  9 +++++++++
>   2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
> index 4b91072f3a4e..83a3d49535e5 100644
> --- a/drivers/tty/vt/vt_ioctl.c
> +++ b/drivers/tty/vt/vt_ioctl.c
> @@ -951,6 +951,22 @@ int vt_ioctl(struct tty_struct *tty,
>   					(unsigned short __user *)arg);
>   	case VT_WAITEVENT:
>   		return vt_event_wait_ioctl((struct vt_event __user *)arg);
> +
> +	case VT_GETCONSIZECSRPOS:
> +	{
> +		struct vt_consizecsrpos concsr;
> +
> +		console_lock();
> +		concsr.con_cols = vc->vc_cols;
> +		concsr.con_rows = vc->vc_rows;
> +		concsr.csr_col = vc->state.x;
> +		concsr.csr_row = vc->state.y;
> +		console_unlock();

Makes a lot of sense!

> +		if (copy_to_user(up, &concsr, sizeof(concsr)))
> +			return -EFAULT;
> +		return 0;
> +	}
> +
>   	default:
>   		return -ENOIOCTLCMD;
>   	}
> diff --git a/include/uapi/linux/vt.h b/include/uapi/linux/vt.h
> index e9d39c48520a..e93c8910133b 100644
> --- a/include/uapi/linux/vt.h
> +++ b/include/uapi/linux/vt.h
> @@ -84,4 +84,13 @@ struct vt_setactivate {
>   
>   #define VT_SETACTIVATE	0x560F	/* Activate and set the mode of a console */
>   
> +struct vt_consizecsrpos {
> +	unsigned short con_rows;	/* number of console rows */
> +	unsigned short con_cols;	/* number of console columns */
> +	unsigned short csr_row;		/* current cursor's row */
> +	unsigned short csr_col;		/* current cursor's column */

Use __u16 pls.

> +};
> +
> +#define VT_GETCONSIZECSRPOS 0x5610  /* get console size and cursor position */

Can we define that properly as
   _IOR(0x56, 0x10, struct vt_consizecsrpos)
? Note this would still differ from "conflicting":
#define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)

thanks,
-- 
js
suse labs

Re: [PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console size and cursor position

[Nicolas Pitre]

On Thu, 15 May 2025, Jiri Slaby wrote:

> On 14. 05. 25, 21:42, Nicolas Pitre wrote:
> > From: Nicolas Pitre <npitre@baylibre.com>
> > 
> > The console dimension and cursor position are available through the
> > /dev/vcsa interface already. However the /dev/vcsa header format uses
> > single-byte fields therefore those values are clamped to 255.
> > 
> > As surprizing as this may seem, some people do use 240-column 67-row
> > screens (a 1920x1080 monitor with 8x16 pixel fonts) which is getting
> > close to the limit. Monitors with higher resolution are not uncommon
> > these days (3840x2160 producing a 480x135 character display) and it is
> > just a matter of time before someone with, say, a braille display using
> > the Linux VT console and BRLTTY on such a screen reports a bug about
> > missing and oddly misaligned screen content.
> > 
> > Let's add VT_GETCONSIZECSRPOS for the retrieval of console size and cursor
> > position without byte-sized limitations. The actual console size limit as
> > encoded in vt.c is 32767x32767 so using a short here is appropriate. Then
> > this can be used to get the cursor position when /dev/vcsa reports 255.
> > 
> > The screen dimension may already be obtained using TIOCGWINSZ and adding
> > the same information to VT_GETCONSIZECSRPOS might be redundant. However
> > applications that care about cursor position also care about display
> > size and having 2 separate system calls to obtain them separately is
> > wasteful. Also, the cursor position can be queried by writing "\e[6n" to
> > a tty and reading back the result but that may be done only by the actual
> > application using that tty and not a sideline observer.
> > 
> > Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> > ---
> >   drivers/tty/vt/vt_ioctl.c | 16 ++++++++++++++++
> >   include/uapi/linux/vt.h   |  9 +++++++++
> >   2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
> > index 4b91072f3a4e..83a3d49535e5 100644
> > --- a/drivers/tty/vt/vt_ioctl.c
> > +++ b/drivers/tty/vt/vt_ioctl.c
> > @@ -951,6 +951,22 @@ int vt_ioctl(struct tty_struct *tty,
> >    				(unsigned short __user *)arg);
> >    case VT_WAITEVENT:
> >   		return vt_event_wait_ioctl((struct vt_event __user *)arg);
> > +
> > +	case VT_GETCONSIZECSRPOS:
> > +	{
> > +		struct vt_consizecsrpos concsr;
> > +
> > +		console_lock();
> > +		concsr.con_cols = vc->vc_cols;
> > +		concsr.con_rows = vc->vc_rows;
> > +		concsr.csr_col = vc->state.x;
> > +		concsr.csr_row = vc->state.y;
> > +		console_unlock();
> 
> Makes a lot of sense!
> 
> > +		if (copy_to_user(up, &concsr, sizeof(concsr)))
> > +			return -EFAULT;
> > +		return 0;
> > +	}
> > +
> >    default:
> >    	return -ENOIOCTLCMD;
> >   	}
> > diff --git a/include/uapi/linux/vt.h b/include/uapi/linux/vt.h
> > index e9d39c48520a..e93c8910133b 100644
> > --- a/include/uapi/linux/vt.h
> > +++ b/include/uapi/linux/vt.h
> > @@ -84,4 +84,13 @@ struct vt_setactivate {
> >   
> >   #define VT_SETACTIVATE	0x560F	/* Activate and set the mode of a
> >   console */
> >   
> > +struct vt_consizecsrpos {
> > +	unsigned short con_rows;	/* number of console rows */
> > +	unsigned short con_cols;	/* number of console columns */
> > +	unsigned short csr_row;		/* current cursor's row */
> > +	unsigned short csr_col;		/* current cursor's column */
> 
> Use __u16 pls.

I beg to differ. Not because __u16 is fundamentally wrong. But 
everything else in this file uses only basic C types already and adding 
one struct with __u16 would look odd. And adding some include to define 
that type would be needed since there are currently no such includes in 
that file currently, and that could potentially cause issues with 
existing consumers of that header file that didn't expect extra 
definitions, etc. So I think that such a change, if it is to happen, 
should be done for the whole file at once and in a separate patch.

> > +};
> > +
> > +#define VT_GETCONSIZECSRPOS 0x5610  /* get console size and cursor position
> > */
> 
> Can we define that properly as
>   _IOR(0x56, 0x10, struct vt_consizecsrpos)
> ? Note this would still differ from "conflicting":
> #define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)

Similarly as the reason above: given that no other definitions in that 
file use the _IO*() scheme for historical reasons, it is preferable to 
follow what's already there to avoid unsuspected confusion. The VT layer 
is pretty much unlykely to grow many additional ioctls in the 
foreseeable future so I'd lean towards keeping things simple and in line 
with the existing code.


Nicolas

Re: [PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console size and cursor position

[Jiri Slaby]

On 15. 05. 25, 18:02, Nicolas Pitre wrote:
> On Thu, 15 May 2025, Jiri Slaby wrote:
> 
>> On 14. 05. 25, 21:42, Nicolas Pitre wrote:
>>> From: Nicolas Pitre <npitre@baylibre.com>
>>>
>>> The console dimension and cursor position are available through the
>>> /dev/vcsa interface already. However the /dev/vcsa header format uses
>>> single-byte fields therefore those values are clamped to 255.
>>>
>>> As surprizing as this may seem, some people do use 240-column 67-row
>>> screens (a 1920x1080 monitor with 8x16 pixel fonts) which is getting
>>> close to the limit. Monitors with higher resolution are not uncommon
>>> these days (3840x2160 producing a 480x135 character display) and it is
>>> just a matter of time before someone with, say, a braille display using
>>> the Linux VT console and BRLTTY on such a screen reports a bug about
>>> missing and oddly misaligned screen content.
>>>
>>> Let's add VT_GETCONSIZECSRPOS for the retrieval of console size and cursor
>>> position without byte-sized limitations. The actual console size limit as
>>> encoded in vt.c is 32767x32767 so using a short here is appropriate. Then
>>> this can be used to get the cursor position when /dev/vcsa reports 255.
>>>
>>> The screen dimension may already be obtained using TIOCGWINSZ and adding
>>> the same information to VT_GETCONSIZECSRPOS might be redundant. However
>>> applications that care about cursor position also care about display
>>> size and having 2 separate system calls to obtain them separately is
>>> wasteful. Also, the cursor position can be queried by writing "\e[6n" to
>>> a tty and reading back the result but that may be done only by the actual
>>> application using that tty and not a sideline observer.
>>>
>>> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
>>> ---
>>>    drivers/tty/vt/vt_ioctl.c | 16 ++++++++++++++++
>>>    include/uapi/linux/vt.h   |  9 +++++++++
>>>    2 files changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
>>> index 4b91072f3a4e..83a3d49535e5 100644
>>> --- a/drivers/tty/vt/vt_ioctl.c
>>> +++ b/drivers/tty/vt/vt_ioctl.c
>>> @@ -951,6 +951,22 @@ int vt_ioctl(struct tty_struct *tty,
>>>     				(unsigned short __user *)arg);
>>>     case VT_WAITEVENT:
>>>    		return vt_event_wait_ioctl((struct vt_event __user *)arg);
>>> +
>>> +	case VT_GETCONSIZECSRPOS:
>>> +	{
>>> +		struct vt_consizecsrpos concsr;
>>> +
>>> +		console_lock();
>>> +		concsr.con_cols = vc->vc_cols;
>>> +		concsr.con_rows = vc->vc_rows;
>>> +		concsr.csr_col = vc->state.x;
>>> +		concsr.csr_row = vc->state.y;
>>> +		console_unlock();
>>
>> Makes a lot of sense!
>>
>>> +		if (copy_to_user(up, &concsr, sizeof(concsr)))
>>> +			return -EFAULT;
>>> +		return 0;
>>> +	}
>>> +
>>>     default:
>>>     	return -ENOIOCTLCMD;
>>>    	}
>>> diff --git a/include/uapi/linux/vt.h b/include/uapi/linux/vt.h
>>> index e9d39c48520a..e93c8910133b 100644
>>> --- a/include/uapi/linux/vt.h
>>> +++ b/include/uapi/linux/vt.h
>>> @@ -84,4 +84,13 @@ struct vt_setactivate {
>>>    
>>>    #define VT_SETACTIVATE	0x560F	/* Activate and set the mode of a
>>>    console */
>>>    
>>> +struct vt_consizecsrpos {
>>> +	unsigned short con_rows;	/* number of console rows */
>>> +	unsigned short con_cols;	/* number of console columns */
>>> +	unsigned short csr_row;		/* current cursor's row */
>>> +	unsigned short csr_col;		/* current cursor's column */
>>
>> Use __u16 pls.
> 
> I beg to differ. Not because __u16 is fundamentally wrong.

Fundamentaly wrong -- for what reason? These types are exactly what 
should be used in userspace APIs instead of bare C types.

> But
> everything else in this file uses only basic C types already and adding
> one struct with __u16 would look odd.

The whole file needs to be fixed, eventually. It's no reason to add 
another bad entry.

> And adding some include to define
> that type would be needed since there are currently no such includes in
> that file currently, and that could potentially cause issues with
> existing consumers of that header file that didn't expect extra
> definitions, etc.

On one side yes, on the other side, sticking with ancient coding style 
while being afraid to include basic headers would be moot.

> So I think that such a change, if it is to happen,
> should be done for the whole file at once and in a separate patch.

Let me bite the bullet and send something. (Likely on Mon -- now queued 
up in my queue for build tests).

>>> +};
>>> +
>>> +#define VT_GETCONSIZECSRPOS 0x5610  /* get console size and cursor position
>>> */
>>
>> Can we define that properly as
>>    _IOR(0x56, 0x10, struct vt_consizecsrpos)
>> ? Note this would still differ from "conflicting":
>> #define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)
> 
> Similarly as the reason above: given that no other definitions in that
> file use the _IO*() scheme for historical reasons, it is preferable to
> follow what's already there to avoid unsuspected confusion. The VT layer
> is pretty much unlykely to grow many additional ioctls in the
> foreseeable future so I'd lean towards keeping things simple and in line
> with the existing code.

I tend to disagree. We should not follow bad practices only because they 
are already there.

-- 
js
suse labs

Re: [PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console size and cursor position

[Nicolas Pitre]

On Fri, 16 May 2025, Jiri Slaby wrote:

> On 15. 05. 25, 18:02, Nicolas Pitre wrote:
> > On Thu, 15 May 2025, Jiri Slaby wrote:
> > 
> >> On 14. 05. 25, 21:42, Nicolas Pitre wrote:
> >>> From: Nicolas Pitre <npitre@baylibre.com>
> >>>
> >>> The console dimension and cursor position are available through the
> >>> /dev/vcsa interface already. However the /dev/vcsa header format uses
> >>> single-byte fields therefore those values are clamped to 255.
> >>>
> >>> As surprizing as this may seem, some people do use 240-column 67-row
> >>> screens (a 1920x1080 monitor with 8x16 pixel fonts) which is getting
> >>> close to the limit. Monitors with higher resolution are not uncommon
> >>> these days (3840x2160 producing a 480x135 character display) and it is
> >>> just a matter of time before someone with, say, a braille display using
> >>> the Linux VT console and BRLTTY on such a screen reports a bug about
> >>> missing and oddly misaligned screen content.
> >>>
> >>> Let's add VT_GETCONSIZECSRPOS for the retrieval of console size and cursor
> >>> position without byte-sized limitations. The actual console size limit as
> >>> encoded in vt.c is 32767x32767 so using a short here is appropriate. Then
> >>> this can be used to get the cursor position when /dev/vcsa reports 255.
> >>>
> >>> The screen dimension may already be obtained using TIOCGWINSZ and adding
> >>> the same information to VT_GETCONSIZECSRPOS might be redundant. However
> >>> applications that care about cursor position also care about display
> >>> size and having 2 separate system calls to obtain them separately is
> >>> wasteful. Also, the cursor position can be queried by writing "\e[6n" to
> >>> a tty and reading back the result but that may be done only by the actual
> >>> application using that tty and not a sideline observer.
> >>>
> >>> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> >>> ---
> >>>    drivers/tty/vt/vt_ioctl.c | 16 ++++++++++++++++
> >>>    include/uapi/linux/vt.h   |  9 +++++++++
> >>>    2 files changed, 25 insertions(+)
> >>>
> >>> diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
> >>> index 4b91072f3a4e..83a3d49535e5 100644
> >>> --- a/drivers/tty/vt/vt_ioctl.c
> >>> +++ b/drivers/tty/vt/vt_ioctl.c
> >>> @@ -951,6 +951,22 @@ int vt_ioctl(struct tty_struct *tty,
> >>>     				(unsigned short __user *)arg);
> >>>     case VT_WAITEVENT:
> >>>    		return vt_event_wait_ioctl((struct vt_event __user *)arg);
> >>> +
> >>> +	case VT_GETCONSIZECSRPOS:
> >>> +	{
> >>> +		struct vt_consizecsrpos concsr;
> >>> +
> >>> +		console_lock();
> >>> +		concsr.con_cols = vc->vc_cols;
> >>> +		concsr.con_rows = vc->vc_rows;
> >>> +		concsr.csr_col = vc->state.x;
> >>> +		concsr.csr_row = vc->state.y;
> >>> +		console_unlock();
> >>
> >> Makes a lot of sense!
> >>
> >>> +		if (copy_to_user(up, &concsr, sizeof(concsr)))
> >>> +			return -EFAULT;
> >>> +		return 0;
> >>> +	}
> >>> +
> >>>     default:
> >>>     	return -ENOIOCTLCMD;
> >>>    	}
> >>> diff --git a/include/uapi/linux/vt.h b/include/uapi/linux/vt.h
> >>> index e9d39c48520a..e93c8910133b 100644
> >>> --- a/include/uapi/linux/vt.h
> >>> +++ b/include/uapi/linux/vt.h
> >>> @@ -84,4 +84,13 @@ struct vt_setactivate {
> >>>    
> >>>    #define VT_SETACTIVATE	0x560F	/* Activate and set the mode of a
> >>>    console */
> >>>    
> >>> +struct vt_consizecsrpos {
> >>> +	unsigned short con_rows;	/* number of console rows */
> >>> +	unsigned short con_cols;	/* number of console columns */
> >>> +	unsigned short csr_row;		/* current cursor's row */
> >>> +	unsigned short csr_col;		/* current cursor's column */
> >>
> >> Use __u16 pls.
> > 
> > I beg to differ. Not because __u16 is fundamentally wrong.
> 
> Fundamentaly wrong -- for what reason? These types are exactly what should be
> used in userspace APIs instead of bare C types.

Sorry, I meant "__u16 is not wrong fundamentally but..."

> > But
> > everything else in this file uses only basic C types already and adding
> > one struct with __u16 would look odd.
> 
> The whole file needs to be fixed, eventually. It's no reason to add another
> bad entry.

Nothing is actually _broken_ now, is it?

> > And adding some include to define
> > that type would be needed since there are currently no such includes in
> > that file currently, and that could potentially cause issues with
> > existing consumers of that header file that didn't expect extra
> > definitions, etc.
> 
> On one side yes, on the other side, sticking with ancient coding style while
> being afraid to include basic headers would be moot.
> 
> > So I think that such a change, if it is to happen,
> > should be done for the whole file at once and in a separate patch.
> 
> Let me bite the bullet and send something. (Likely on Mon -- now queued up in
> my queue for build tests).
> 
> >>> +};
> >>> +
> >>> +#define VT_GETCONSIZECSRPOS 0x5610  /* get console size and cursor
> >>> position
> >>> */
> >>
> >> Can we define that properly as
> >>    _IOR(0x56, 0x10, struct vt_consizecsrpos)
> >> ? Note this would still differ from "conflicting":
> >> #define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)
> > 
> > Similarly as the reason above: given that no other definitions in that
> > file use the _IO*() scheme for historical reasons, it is preferable to
> > follow what's already there to avoid unsuspected confusion. The VT layer
> > is pretty much unlykely to grow many additional ioctls in the
> > foreseeable future so I'd lean towards keeping things simple and in line
> > with the existing code.
> 
> I tend to disagree. We should not follow bad practices only because they are
> already there.

Sometimes it is better to leave things as they are, especially when 
they've been static and stable for so long, as disturbing stuff in the 
name of removing "bad practices" may bring unforeseen consequences. 
Perfect is the enemy of good 
(https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good).


Nicolas

Re: [PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console size and cursor position

[Nicolas Pitre]

On Fri, 16 May 2025, Jiri Slaby wrote:

> On 15. 05. 25, 18:02, Nicolas Pitre wrote:
> > So I think that such a change, if it is to happen,
> > should be done for the whole file at once and in a separate patch.
> 
> Let me bite the bullet and send something. (Likely on Mon -- now queued up in
> my queue for build tests).

Do you have that patch ready? I'd be happy to rebase my changes on top 
of it and adapt them to follow the file's latest code style.


Nicolas

Re: [PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console size and cursor position

[Nicolas Pitre]

On Thu, 15 May 2025, Nicolas Pitre wrote:

> On Thu, 15 May 2025, Jiri Slaby wrote:
> 
> > On 14. 05. 25, 21:42, Nicolas Pitre wrote:
> > > +#define VT_GETCONSIZECSRPOS 0x5610  /* get console size and cursor position
> > > */
> > 
> > Can we define that properly as
> >   _IOR(0x56, 0x10, struct vt_consizecsrpos)
> > ? Note this would still differ from "conflicting":
> > #define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)
> 
> Similarly as the reason above: given that no other definitions in that 
> file use the _IO*() scheme for historical reasons, it is preferable to 
> follow what's already there to avoid unsuspected confusion. The VT layer 
> is pretty much unlykely to grow many additional ioctls in the 
> foreseeable future so I'd lean towards keeping things simple and in line 
> with the existing code.

OK... I found precedents for a mixed sheme in 
include/uapi/asm-generic/ioctls.h. Therefore I give in.


Nicolas