[PATCH 1/2] fbconsole: check cursor position before moving

[PATCH 1/2] fbconsole: check cursor position before moving

[Bastian Stender]

Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
console lead to a barebox crash while drawing the cursor. The cursor can
only be moved to a valid position between (0,0) and (priv->cols,
priv->rows) now.

Signed-off-by: Bastian Stender <bst@pengutronix.de>
---
 drivers/video/fbconsole.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
index 64f7d7364e..3879741b2a 100644
--- a/drivers/video/fbconsole.c
+++ b/drivers/video/fbconsole.c
@@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
 		return;
 	case 'H':
 		video_invertchar(priv, priv->x, priv->y);
+
 		pos = simple_strtoul(priv->csi, &end, 10);
-		priv->y = pos ? pos - 1 : 0;
+		priv->y = (pos && pos <= priv->rows + 1) ? pos - 1 : 0;
+
 		pos = simple_strtoul(end + 1, NULL, 10);
-		priv->x = pos ? pos - 1 : 0;
+		priv->x = (pos && pos <= priv->cols + 1) ? pos - 1 : 0;
+
 		video_invertchar(priv, priv->x, priv->y);
 	case 'K':
 		pos = simple_strtoul(priv->csi, &end, 10);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] fbconsole: implement vt100 cursor shown/hidden

[Bastian Stender]

This implements the vt100 show cursor command '[?25h' and the hide
cursor command '[?25l'. It is useful for displaying text on a non-active
(no stdout/stdin/stderr) console with 'echo'.

Signed-off-by: Bastian Stender <bst@pengutronix.de>
---
 drivers/video/fbconsole.c | 55 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
index 3879741b2a..b6059e86f5 100644
--- a/drivers/video/fbconsole.c
+++ b/drivers/video/fbconsole.c
@@ -11,6 +11,7 @@ enum state_t {
 	LIT,				/* Literal input */
 	ESC,				/* Start of escape sequence */
 	CSI,				/* Reading arguments in "CSI Pn ;...*/
+	CSI_CNT,
 };
 
 struct fbc_priv {
@@ -34,10 +35,12 @@ struct fbc_priv {
 
 #define ANSI_FLAG_INVERT	(1 << 0)
 #define ANSI_FLAG_BRIGHT	(1 << 1)
+#define HIDE_CURSOR		(1 << 2)
 	unsigned flags;
 
 	int csipos;
 	u8 csi[256];
+	unsigned char csi_cmd;
 
 	int active;
 	int in_console;
@@ -144,6 +147,12 @@ static void video_invertchar(struct fbc_priv *priv, int x, int y)
 			priv->font->width, priv->font->height);
 }
 
+static void show_cursor(struct fbc_priv *priv, int x, int y)
+{
+	if (!(priv->flags & HIDE_CURSOR))
+		video_invertchar(priv, x, y);
+}
+
 static void printchar(struct fbc_priv *priv, int c)
 {
 	video_invertchar(priv, priv->x, priv->y);
@@ -200,7 +209,7 @@ static void printchar(struct fbc_priv *priv, int c)
 		priv->y = priv->rows;
 	}
 
-	video_invertchar(priv, priv->x, priv->y);
+	show_cursor(priv, priv->x, priv->y);
 
 	return;
 }
@@ -258,12 +267,43 @@ static void fbc_parse_csi(struct fbc_priv *priv)
 	case 'm':
 		fbc_parse_colors(priv);
 		return;
+	case '?': /* vt100: show/hide cursor */
+		priv->csi_cmd = last;
+		priv->state = CSI_CNT;
+		return;
+	case 'h':
+		/* suffix for vt100 "[?25h" */
+		switch (priv->csi_cmd) {
+		case '?': /* cursor visible */
+			priv->csi_cmd = -1;
+			if (!(priv->flags & HIDE_CURSOR))
+				break;
+
+			priv->flags &= ~HIDE_CURSOR;
+			/* show cursor now */
+			show_cursor(priv, priv->x, priv->y);
+			break;
+		}
+		break;
+	case 'l':
+		/* suffix for vt100 "[?25l" */
+		switch (priv->csi_cmd) {
+		case '?': /* cursor invisible */
+			priv->csi_cmd = -1;
+
+			/* hide cursor now */
+			video_invertchar(priv, priv->x, priv->y);
+			priv->flags |= HIDE_CURSOR;
+
+			break;
+		}
+		break;
 	case 'J':
 		cls(priv);
-		video_invertchar(priv, priv->x, priv->y);
+		show_cursor(priv, priv->x, priv->y);
 		return;
 	case 'H':
-		video_invertchar(priv, priv->x, priv->y);
+		show_cursor(priv, priv->x, priv->y);
 
 		pos = simple_strtoul(priv->csi, &end, 10);
 		priv->y = (pos && pos <= priv->rows + 1) ? pos - 1 : 0;
@@ -271,7 +311,7 @@ static void fbc_parse_csi(struct fbc_priv *priv)
 		pos = simple_strtoul(end + 1, NULL, 10);
 		priv->x = (pos && pos <= priv->cols + 1) ? pos - 1 : 0;
 
-		video_invertchar(priv, priv->x, priv->y);
+		show_cursor(priv, priv->x, priv->y);
 	case 'K':
 		pos = simple_strtoul(priv->csi, &end, 10);
 		video_invertchar(priv, priv->x, priv->y);
@@ -343,9 +383,14 @@ static void fbc_putc(struct console_device *cdev, char c)
 			break;
 		default:
 			fbc_parse_csi(priv);
-			priv->state = LIT;
+			if (priv->state != CSI_CNT)
+				priv->state = LIT;
 		}
 		break;
+	case CSI_CNT:
+		priv->state = CSI;
+		break;
+
 	}
 	priv->in_console = 0;
 }
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] fbconsole: check cursor position before moving

[Sascha Hauer]

On Thu, Feb 23, 2017 at 06:20:21PM +0100, Bastian Stender wrote:
> Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
> console lead to a barebox crash while drawing the cursor. The cursor can
> only be moved to a valid position between (0,0) and (priv->cols,
> priv->rows) now.
> 
> Signed-off-by: Bastian Stender <bst@pengutronix.de>
> ---
>  drivers/video/fbconsole.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
> index 64f7d7364e..3879741b2a 100644
> --- a/drivers/video/fbconsole.c
> +++ b/drivers/video/fbconsole.c
> @@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
>  		return;
>  	case 'H':
>  		video_invertchar(priv, priv->x, priv->y);
> +
>  		pos = simple_strtoul(priv->csi, &end, 10);
> -		priv->y = pos ? pos - 1 : 0;
> +		priv->y = (pos && pos <= priv->rows + 1) ? pos - 1 : 0;
> +
>  		pos = simple_strtoul(end + 1, NULL, 10);
> -		priv->x = pos ? pos - 1 : 0;
> +		priv->x = (pos && pos <= priv->cols + 1) ? pos - 1 : 0;
> +

When moving out of the screen shouldn't we place the cursor on the
bottom right corner of the screen? With this patch we move it to the top
left corner.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] fbconsole: check cursor position before moving

[Ian Abbott]

On 24/02/17 07:32, Sascha Hauer wrote:
> On Thu, Feb 23, 2017 at 06:20:21PM +0100, Bastian Stender wrote:
>> Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
>> console lead to a barebox crash while drawing the cursor. The cursor can
>> only be moved to a valid position between (0,0) and (priv->cols,
>> priv->rows) now.
>>
>> Signed-off-by: Bastian Stender <bst@pengutronix.de>
>> ---
>>  drivers/video/fbconsole.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
>> index 64f7d7364e..3879741b2a 100644
>> --- a/drivers/video/fbconsole.c
>> +++ b/drivers/video/fbconsole.c
>> @@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
>>  		return;
>>  	case 'H':
>>  		video_invertchar(priv, priv->x, priv->y);
>> +
>>  		pos = simple_strtoul(priv->csi, &end, 10);
>> -		priv->y = pos ? pos - 1 : 0;
>> +		priv->y = (pos && pos <= priv->rows + 1) ? pos - 1 : 0;
>> +
>>  		pos = simple_strtoul(end + 1, NULL, 10);
>> -		priv->x = pos ? pos - 1 : 0;
>> +		priv->x = (pos && pos <= priv->cols + 1) ? pos - 1 : 0;
>> +
>
> When moving out of the screen shouldn't we place the cursor on the
> bottom right corner of the screen? With this patch we move it to the top
> left corner.

Or clip to each edge, something like:

  priv->y = !pos ? 0 : pos < priv->rows ? pos - 1 : priv->rows - 1;

Same for priv->x and priv->cols?

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] fbconsole: check cursor position before moving

[Bastian Stender]

On 02/24/2017 12:18 PM, Ian Abbott wrote:
> On 24/02/17 07:32, Sascha Hauer wrote:
>> On Thu, Feb 23, 2017 at 06:20:21PM +0100, Bastian Stender wrote:
>>> Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
>>> console lead to a barebox crash while drawing the cursor. The cursor can
>>> only be moved to a valid position between (0,0) and (priv->cols,
>>> priv->rows) now.
>>>
>>> Signed-off-by: Bastian Stender <bst@pengutronix.de>
>>> ---
>>>  drivers/video/fbconsole.c | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
>>> index 64f7d7364e..3879741b2a 100644
>>> --- a/drivers/video/fbconsole.c
>>> +++ b/drivers/video/fbconsole.c
>>> @@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
>>>          return;
>>>      case 'H':
>>>          video_invertchar(priv, priv->x, priv->y);
>>> +
>>>          pos = simple_strtoul(priv->csi, &end, 10);
>>> -        priv->y = pos ? pos - 1 : 0;
>>> +        priv->y = (pos && pos <= priv->rows + 1) ? pos - 1 : 0;
>>> +
>>>          pos = simple_strtoul(end + 1, NULL, 10);
>>> -        priv->x = pos ? pos - 1 : 0;
>>> +        priv->x = (pos && pos <= priv->cols + 1) ? pos - 1 : 0;
>>> +
>>
>> When moving out of the screen shouldn't we place the cursor on the
>> bottom right corner of the screen? With this patch we move it to the top
>> left corner.
>
> Or clip to each edge, something like:
>
>  priv->y = !pos ? 0 : pos < priv->rows ? pos - 1 : priv->rows - 1;
>
> Same for priv->x and priv->cols?

Sounds good to me, I'll send a new version of this patch.

Regards,
Bastian

-- 
Pengutronix e.K.
Industrial Linux Solutions
http://www.pengutronix.de/
Peiner Str. 6-8, 31137 Hildesheim, Germany
Amtsgericht Hildesheim, HRA 2686

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2] fbconsole: check cursor position before moving

[Bastian Stender]

Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
console lead to a barebox crash while drawing the cursor. If the
cursor position is out of bounds clip the cursor to the corresponding
edge.

Signed-off-by: Bastian Stender <bst@pengutronix.de>
---
 drivers/video/fbconsole.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
index 64f7d7364e..843a026ff6 100644
--- a/drivers/video/fbconsole.c
+++ b/drivers/video/fbconsole.c
@@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
 		return;
 	case 'H':
 		video_invertchar(priv, priv->x, priv->y);
+
 		pos = simple_strtoul(priv->csi, &end, 10);
-		priv->y = pos ? pos - 1 : 0;
+		priv->y = !pos ? 0 : pos < priv->rows ? pos - 1 : priv->rows - 1;
+
 		pos = simple_strtoul(end + 1, NULL, 10);
-		priv->x = pos ? pos - 1 : 0;
+		priv->x = !pos ? 0 : pos < priv->cols ? pos - 1 : priv->cols - 1;
+
 		video_invertchar(priv, priv->x, priv->y);
 	case 'K':
 		pos = simple_strtoul(priv->csi, &end, 10);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2] fbconsole: check cursor position before moving

[Sascha Hauer]

On Fri, Feb 24, 2017 at 02:36:27PM +0100, Bastian Stender wrote:
> Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
> console lead to a barebox crash while drawing the cursor. If the
> cursor position is out of bounds clip the cursor to the corresponding
> edge.
> 
> Signed-off-by: Bastian Stender <bst@pengutronix.de>
> ---
>  drivers/video/fbconsole.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
> index 64f7d7364e..843a026ff6 100644
> --- a/drivers/video/fbconsole.c
> +++ b/drivers/video/fbconsole.c
> @@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
>  		return;
>  	case 'H':
>  		video_invertchar(priv, priv->x, priv->y);
> +
>  		pos = simple_strtoul(priv->csi, &end, 10);
> -		priv->y = pos ? pos - 1 : 0;
> +		priv->y = !pos ? 0 : pos < priv->rows ? pos - 1 : priv->rows - 1;
> +
>  		pos = simple_strtoul(end + 1, NULL, 10);
> -		priv->x = pos ? pos - 1 : 0;
> +		priv->x = !pos ? 0 : pos < priv->cols ? pos - 1 : priv->cols - 1;

Not nice to read. Fortunately this situation is common enough so that we
have a function for it:

		priv->x = clamp(pos, 0, priv->cols - 1);

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3] fbconsole: check cursor position before moving

[Bastian Stender]

Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
console lead to a barebox crash while drawing the cursor. If the
cursor position is out of bounds clip the cursor to the corresponding
edge.

Signed-off-by: Bastian Stender <bst@pengutronix.de>
---
 drivers/video/fbconsole.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
index 64f7d7364e..33649c597d 100644
--- a/drivers/video/fbconsole.c
+++ b/drivers/video/fbconsole.c
@@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
 		return;
 	case 'H':
 		video_invertchar(priv, priv->x, priv->y);
+
 		pos = simple_strtoul(priv->csi, &end, 10);
-		priv->y = pos ? pos - 1 : 0;
+		priv->y = clamp(pos - 1, 0, (int) priv->rows);
+
 		pos = simple_strtoul(end + 1, NULL, 10);
-		priv->x = pos ? pos - 1 : 0;
+		priv->x = clamp(pos - 1, 0, (int) priv->cols);
+
 		video_invertchar(priv, priv->x, priv->y);
 	case 'K':
 		pos = simple_strtoul(priv->csi, &end, 10);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3] fbconsole: check cursor position before moving

[Ian Abbott]

On 27/02/17 13:39, Bastian Stender wrote:
> Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
> console lead to a barebox crash while drawing the cursor. If the
> cursor position is out of bounds clip the cursor to the corresponding
> edge.
>
> Signed-off-by: Bastian Stender <bst@pengutronix.de>
> ---
>  drivers/video/fbconsole.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
> index 64f7d7364e..33649c597d 100644
> --- a/drivers/video/fbconsole.c
> +++ b/drivers/video/fbconsole.c
> @@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
>  		return;
>  	case 'H':
>  		video_invertchar(priv, priv->x, priv->y);
> +
>  		pos = simple_strtoul(priv->csi, &end, 10);
> -		priv->y = pos ? pos - 1 : 0;
> +		priv->y = clamp(pos - 1, 0, (int) priv->rows);
> +

That allows priv->y to be set to priv->rows, which is one too many.

How about:

		priv->y = clamp(pos, 1, (int)priv->rows) - 1;

?

>  		pos = simple_strtoul(end + 1, NULL, 10);
> -		priv->x = pos ? pos - 1 : 0;
> +		priv->x = clamp(pos - 1, 0, (int) priv->cols);
> +

Similar to above.

>  		video_invertchar(priv, priv->x, priv->y);
>  	case 'K':
>  		pos = simple_strtoul(priv->csi, &end, 10);
>


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3] fbconsole: check cursor position before moving

[Ian Abbott]

On 27/02/17 18:46, Ian Abbott wrote:
> On 27/02/17 13:39, Bastian Stender wrote:
>> Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
>> console lead to a barebox crash while drawing the cursor. If the
>> cursor position is out of bounds clip the cursor to the corresponding
>> edge.
>>
>> Signed-off-by: Bastian Stender <bst@pengutronix.de>
>> ---
>>  drivers/video/fbconsole.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
>> index 64f7d7364e..33649c597d 100644
>> --- a/drivers/video/fbconsole.c
>> +++ b/drivers/video/fbconsole.c
>> @@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
>>          return;
>>      case 'H':
>>          video_invertchar(priv, priv->x, priv->y);
>> +
>>          pos = simple_strtoul(priv->csi, &end, 10);
>> -        priv->y = pos ? pos - 1 : 0;
>> +        priv->y = clamp(pos - 1, 0, (int) priv->rows);
>> +
>
> That allows priv->y to be set to priv->rows, which is one too many.

Ignore that.  My mistake.  It appears that priv->rows is the number of 
rows minus 1, so the above code is fine.  Same for priv->x.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3] fbconsole: check cursor position before moving

[Sascha Hauer]

On Mon, Feb 27, 2017 at 02:39:30PM +0100, Bastian Stender wrote:
> Moving the cursor to x=2, y=2 with "\e[3;3H" on a 12x2 framebuffer
> console lead to a barebox crash while drawing the cursor. If the
> cursor position is out of bounds clip the cursor to the corresponding
> edge.
> 
> Signed-off-by: Bastian Stender <bst@pengutronix.de>

Applied, thanks

Sascha

> ---
>  drivers/video/fbconsole.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
> index 64f7d7364e..33649c597d 100644
> --- a/drivers/video/fbconsole.c
> +++ b/drivers/video/fbconsole.c
> @@ -264,10 +264,13 @@ static void fbc_parse_csi(struct fbc_priv *priv)
>  		return;
>  	case 'H':
>  		video_invertchar(priv, priv->x, priv->y);
> +
>  		pos = simple_strtoul(priv->csi, &end, 10);
> -		priv->y = pos ? pos - 1 : 0;
> +		priv->y = clamp(pos - 1, 0, (int) priv->rows);
> +
>  		pos = simple_strtoul(end + 1, NULL, 10);
> -		priv->x = pos ? pos - 1 : 0;
> +		priv->x = clamp(pos - 1, 0, (int) priv->cols);
> +
>  		video_invertchar(priv, priv->x, priv->y);
>  	case 'K':
>  		pos = simple_strtoul(priv->csi, &end, 10);
> -- 
> 2.11.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox