[RFC PATCH] fbconsole: add missing break

[RFC PATCH] fbconsole: add missing break

[Jules Maselbas]

The cursor move CSI has nothing to do with the clear-screen CSI.
The existing fallthrough looks like a bug, add the missing break.

Fixes: 27f79c05ab ("video: implement framebuffer console")
Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
---

 I am not 100% sure if this is a bug or not, it feels like it sould not
 fallthrough, an extra look would be nice.


 drivers/video/fbconsole.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
index 24592d00ed..7462e34778 100644
--- a/drivers/video/fbconsole.c
+++ b/drivers/video/fbconsole.c
@@ -576,6 +576,7 @@ static void fbc_parse_csi(struct fbc_priv *priv)
 		priv->x = clamp(pos - 1, 0, (int) priv->cols - 1);
 
 		show_cursor(priv, priv->x, priv->y);
+		break;
 	case 'K':
 		pos = simple_strtoul(priv->csi, &end, 10);
 		video_invertchar(priv, priv->x, priv->y);
-- 
2.48.1

Re: [RFC PATCH] fbconsole: add missing break

[Sascha Hauer]

On Tue, 25 Mar 2025 17:00:08 +0100, Jules Maselbas wrote:
> The cursor move CSI has nothing to do with the clear-screen CSI.
> The existing fallthrough looks like a bug, add the missing break.
> 
> 

Applied, thanks!

[1/1] fbconsole: add missing break
      https://git.pengutronix.de/cgit/barebox/commit/?id=5b2e47b1ff4c (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [RFC PATCH] fbconsole: add missing break

[Sascha Hauer]

On Tue, Mar 25, 2025 at 05:00:08PM +0100, Jules Maselbas wrote:
> The cursor move CSI has nothing to do with the clear-screen CSI.

CSI n K is erase to beginning or end of line.

> The existing fallthrough looks like a bug, add the missing break.
> 
> Fixes: 27f79c05ab ("video: implement framebuffer console")
> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
> ---
> 
>  I am not 100% sure if this is a bug or not, it feels like it sould not
>  fallthrough, an extra look would be nice.

Yes, really looks like a bug. Moving the cursor shouldn't erase
anything.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [RFC PATCH] fbconsole: add missing break

[Ahmad Fatoum]

On 25.03.25 17:00, Jules Maselbas wrote:
> The cursor move CSI has nothing to do with the clear-screen CSI.
> The existing fallthrough looks like a bug, add the missing break.
> 
> Fixes: 27f79c05ab ("video: implement framebuffer console")
> Signed-off-by: Jules Maselbas <jmaselbas@zdiv.net>
> ---
> 
>  I am not 100% sure if this is a bug or not, it feels like it sould not
>  fallthrough, an extra look would be nice.

I think it makes sense to add the break.

case 'H' (\e[{line};{column}H) is move cursor and
case 'K' (ESC[{0,1,2}K} is erase.

The code currently interprets the same integer once as line
and another time as erase type for K, which is not lines,
but rather 0 = from cursor to end of line,
           1 = from start of line to cursor
           2 = whole line 

So this patch looks correct IMO.

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> 
> 
>  drivers/video/fbconsole.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
> index 24592d00ed..7462e34778 100644
> --- a/drivers/video/fbconsole.c
> +++ b/drivers/video/fbconsole.c
> @@ -576,6 +576,7 @@ static void fbc_parse_csi(struct fbc_priv *priv)
>  		priv->x = clamp(pos - 1, 0, (int) priv->cols - 1);
>  
>  		show_cursor(priv, priv->x, priv->y);
> +		break;
>  	case 'K':
>  		pos = simple_strtoul(priv->csi, &end, 10);
>  		video_invertchar(priv, priv->x, priv->y);


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |