linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check()
@ 2023-01-12  8:01 Jiri Slaby (SUSE)
  2023-01-12  8:01 ` [PATCH 02/11] tty: vt: drop get_vc_uniscr() Jiri Slaby (SUSE)
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

VC_UNI_SCREEN_DEBUG is always defined as 0, so this code is never
executed. Drop it along with VC_UNI_SCREEN_DEBUG.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 40 ----------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 981d2bfcf9a5..4b804665c51f 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -323,8 +323,6 @@ void schedule_console_callback(void)
 #define get_vc_uniscr(vc) vc->vc_uni_screen
 #endif
 
-#define VC_UNI_SCREEN_DEBUG 0
-
 typedef uint32_t char32_t;
 
 /*
@@ -580,43 +578,6 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 	}
 }
 
-/* this is for validation and debugging only */
-static void vc_uniscr_debug_check(struct vc_data *vc)
-{
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
-	unsigned short *p;
-	int x, y, mask;
-
-	if (!VC_UNI_SCREEN_DEBUG || !uniscr)
-		return;
-
-	WARN_CONSOLE_UNLOCKED();
-
-	/*
-	 * Make sure our unicode screen translates into the same glyphs
-	 * as the actual screen. This is brutal indeed.
-	 */
-	p = (unsigned short *)vc->vc_origin;
-	mask = vc->vc_hi_font_mask | 0xff;
-	for (y = 0; y < vc->vc_rows; y++) {
-		char32_t *line = uniscr->lines[y];
-		for (x = 0; x < vc->vc_cols; x++) {
-			u16 glyph = scr_readw(p++) & mask;
-			char32_t uc = line[x];
-			int tc = conv_uni_to_pc(vc, uc);
-			if (tc == -4)
-				tc = conv_uni_to_pc(vc, 0xfffd);
-			if (tc == -4)
-				tc = conv_uni_to_pc(vc, '?');
-			if (tc != glyph)
-				pr_err_ratelimited(
-					"%s: mismatch at %d,%d: glyph=%#x tc=%#x\n",
-					__func__, x, y, glyph, tc);
-		}
-	}
-}
-
-
 static void con_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
 		enum con_scroll dir, unsigned int nr)
 {
@@ -2959,7 +2920,6 @@ static int do_con_write(struct tty_struct *tty, const unsigned char *buf, int co
 			goto rescan_last_byte;
 	}
 	con_flush(vc, &draw);
-	vc_uniscr_debug_check(vc);
 	console_conditional_schedule();
 	notify_update(vc);
 	console_unlock();
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/11] tty: vt: drop get_vc_uniscr()
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12  8:41   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 03/11] tty: vt: remove reference to undefined NO_VC_UNI_SCREEN Jiri Slaby (SUSE)
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

Its definition depends on the NO_VC_UNI_SCREEN macro. But that is never
defined, so remove all this completely. I.e. expand the macro to
vc->vc_uni_screen everywhere.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 4b804665c51f..7e5baf9f8ad8 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -316,13 +316,6 @@ void schedule_console_callback(void)
  * Code to manage unicode-based screen buffers
  */
 
-#ifdef NO_VC_UNI_SCREEN
-/* this disables and optimizes related code away at compile time */
-#define get_vc_uniscr(vc) NULL
-#else
-#define get_vc_uniscr(vc) vc->vc_uni_screen
-#endif
-
 typedef uint32_t char32_t;
 
 /*
@@ -369,7 +362,7 @@ static void vc_uniscr_set(struct vc_data *vc, struct uni_screen *new_uniscr)
 
 static void vc_uniscr_putc(struct vc_data *vc, char32_t uc)
 {
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
+	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr)
 		uniscr->lines[vc->state.y][vc->state.x] = uc;
@@ -377,7 +370,7 @@ static void vc_uniscr_putc(struct vc_data *vc, char32_t uc)
 
 static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
 {
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
+	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr) {
 		char32_t *ln = uniscr->lines[vc->state.y];
@@ -390,7 +383,7 @@ static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
 
 static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
 {
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
+	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr) {
 		char32_t *ln = uniscr->lines[vc->state.y];
@@ -404,7 +397,7 @@ static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
 static void vc_uniscr_clear_line(struct vc_data *vc, unsigned int x,
 				 unsigned int nr)
 {
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
+	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr) {
 		char32_t *ln = uniscr->lines[vc->state.y];
@@ -416,7 +409,7 @@ static void vc_uniscr_clear_line(struct vc_data *vc, unsigned int x,
 static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
 				  unsigned int nr)
 {
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
+	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr) {
 		unsigned int cols = vc->vc_cols;
@@ -429,7 +422,7 @@ static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
 static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
 			     enum con_scroll dir, unsigned int nr)
 {
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
+	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr) {
 		unsigned int i, j, k, sz, d, clear;
@@ -545,7 +538,7 @@ int vc_uniscr_check(struct vc_data *vc)
 void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 			 unsigned int row, unsigned int col, unsigned int nr)
 {
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
+	struct uni_screen *uniscr = vc->vc_uni_screen;
 	int offset = row * vc->vc_size_row + col * 2;
 	unsigned long pos;
 
@@ -1206,7 +1199,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 	if (!newscreen)
 		return -ENOMEM;
 
-	if (get_vc_uniscr(vc)) {
+	if (vc->vc_uni_screen) {
 		new_uniscr = vc_uniscr_alloc(new_cols, new_rows);
 		if (!new_uniscr) {
 			kfree(newscreen);
@@ -1258,7 +1251,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 	end = old_origin + old_row_size * min(old_rows, new_rows);
 
 	vc_uniscr_copy_area(new_uniscr, new_cols, new_rows,
-			    get_vc_uniscr(vc), rlth/2, first_copied_row,
+			    vc->vc_uni_screen, rlth/2, first_copied_row,
 			    min(old_rows, new_rows));
 	vc_uniscr_set(vc, new_uniscr);
 
@@ -4700,7 +4693,7 @@ EXPORT_SYMBOL_GPL(screen_glyph);
 
 u32 screen_glyph_unicode(const struct vc_data *vc, int n)
 {
-	struct uni_screen *uniscr = get_vc_uniscr(vc);
+	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr)
 		return uniscr->lines[n / vc->vc_cols][n % vc->vc_cols];
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/11] tty: vt: remove reference to undefined NO_VC_UNI_SCREEN
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
  2023-01-12  8:01 ` [PATCH 02/11] tty: vt: drop get_vc_uniscr() Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12  8:44   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 04/11] tty: vt: use sizeof(*variable) where possible Jiri Slaby (SUSE)
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

NO_VC_UNI_SCREEN is defined nowhere. Remove the last reference to it.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 7e5baf9f8ad8..561c82e120cf 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -495,9 +495,6 @@ int vc_uniscr_check(struct vc_data *vc)
 	unsigned short *p;
 	int x, y, mask;
 
-	if (__is_defined(NO_VC_UNI_SCREEN))
-		return -EOPNOTSUPP;
-
 	WARN_CONSOLE_UNLOCKED();
 
 	if (!vc->vc_utf)
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/11] tty: vt: use sizeof(*variable) where possible
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
  2023-01-12  8:01 ` [PATCH 02/11] tty: vt: drop get_vc_uniscr() Jiri Slaby (SUSE)
  2023-01-12  8:01 ` [PATCH 03/11] tty: vt: remove reference to undefined NO_VC_UNI_SCREEN Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12  8:58   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 05/11] tty: vt: remove char32_t typedef Jiri Slaby (SUSE)
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

Instead of sizeof(type), use sizeof(*variable) which is preferred. We
are going to remove the unicode's char32_t typedef, so this makes the
switch easier.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 561c82e120cf..3ae0212f1aa7 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -330,11 +330,11 @@ static struct uni_screen *vc_uniscr_alloc(unsigned int cols, unsigned int rows)
 {
 	struct uni_screen *uniscr;
 	void *p;
-	unsigned int memsize, i;
+	unsigned int memsize, i, col_size = cols * sizeof(**uniscr->lines);
 
 	/* allocate everything in one go */
-	memsize = cols * rows * sizeof(char32_t);
-	memsize += rows * sizeof(char32_t *);
+	memsize = col_size * rows;
+	memsize += rows * sizeof(*uniscr->lines);
 	p = vzalloc(memsize);
 	if (!p)
 		return NULL;
@@ -344,7 +344,7 @@ static struct uni_screen *vc_uniscr_alloc(unsigned int cols, unsigned int rows)
 	p = uniscr->lines + rows;
 	for (i = 0; i < rows; i++) {
 		uniscr->lines[i] = p;
-		p += cols * sizeof(char32_t);
+		p += col_size;
 	}
 	return uniscr;
 }
@@ -469,7 +469,7 @@ static void vc_uniscr_copy_area(struct uni_screen *dst,
 		char32_t *src_line = src->lines[src_top_row];
 		char32_t *dst_line = dst->lines[dst_row];
 
-		memcpy(dst_line, src_line, src_cols * sizeof(char32_t));
+		memcpy(dst_line, src_line, src_cols * sizeof(*src_line));
 		if (dst_cols - src_cols)
 			memset32(dst_line + src_cols, ' ', dst_cols - src_cols);
 		src_top_row++;
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/11] tty: vt: remove char32_t typedef
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
                   ` (2 preceding siblings ...)
  2023-01-12  8:01 ` [PATCH 04/11] tty: vt: use sizeof(*variable) where possible Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12  8:52   ` Ilpo Järvinen
  2023-01-12  9:34   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 06/11] tty: vt: remove struct uni_screen Jiri Slaby (SUSE)
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

It boils down to uint32_t, so use u32 directly, instead. This makes the
code more obvious.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 3ae0212f1aa7..86c18522231b 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -316,14 +316,12 @@ void schedule_console_callback(void)
  * Code to manage unicode-based screen buffers
  */
 
-typedef uint32_t char32_t;
-
 /*
  * Our screen buffer is preceded by an array of line pointers so that
  * scrolling only implies some pointer shuffling.
  */
 struct uni_screen {
-	char32_t *lines[0];
+	u32 *lines[0];
 };
 
 static struct uni_screen *vc_uniscr_alloc(unsigned int cols, unsigned int rows)
@@ -360,7 +358,7 @@ static void vc_uniscr_set(struct vc_data *vc, struct uni_screen *new_uniscr)
 	vc->vc_uni_screen = new_uniscr;
 }
 
-static void vc_uniscr_putc(struct vc_data *vc, char32_t uc)
+static void vc_uniscr_putc(struct vc_data *vc, u32 uc)
 {
 	struct uni_screen *uniscr = vc->vc_uni_screen;
 
@@ -373,7 +371,7 @@ static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
 	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr) {
-		char32_t *ln = uniscr->lines[vc->state.y];
+		u32 *ln = uniscr->lines[vc->state.y];
 		unsigned int x = vc->state.x, cols = vc->vc_cols;
 
 		memmove(&ln[x + nr], &ln[x], (cols - x - nr) * sizeof(*ln));
@@ -386,7 +384,7 @@ static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
 	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr) {
-		char32_t *ln = uniscr->lines[vc->state.y];
+		u32 *ln = uniscr->lines[vc->state.y];
 		unsigned int x = vc->state.x, cols = vc->vc_cols;
 
 		memcpy(&ln[x], &ln[x + nr], (cols - x - nr) * sizeof(*ln));
@@ -400,7 +398,7 @@ static void vc_uniscr_clear_line(struct vc_data *vc, unsigned int x,
 	struct uni_screen *uniscr = vc->vc_uni_screen;
 
 	if (uniscr) {
-		char32_t *ln = uniscr->lines[vc->state.y];
+		u32 *ln = uniscr->lines[vc->state.y];
 
 		memset32(&ln[x], ' ', nr);
 	}
@@ -435,7 +433,7 @@ static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
 			d = sz - nr;
 		}
 		for (i = 0; i < gcd(d, sz); i++) {
-			char32_t *tmp = uniscr->lines[t + i];
+			u32 *tmp = uniscr->lines[t + i];
 			j = i;
 			while (1) {
 				k = j + d;
@@ -466,8 +464,8 @@ static void vc_uniscr_copy_area(struct uni_screen *dst,
 		return;
 
 	while (src_top_row < src_bot_row) {
-		char32_t *src_line = src->lines[src_top_row];
-		char32_t *dst_line = dst->lines[dst_row];
+		u32 *src_line = src->lines[src_top_row];
+		u32 *dst_line = dst->lines[dst_row];
 
 		memcpy(dst_line, src_line, src_cols * sizeof(*src_line));
 		if (dst_cols - src_cols)
@@ -476,7 +474,7 @@ static void vc_uniscr_copy_area(struct uni_screen *dst,
 		dst_row++;
 	}
 	while (dst_row < dst_rows) {
-		char32_t *dst_line = dst->lines[dst_row];
+		u32 *dst_line = dst->lines[dst_row];
 
 		memset32(dst_line, ' ', dst_cols);
 		dst_row++;
@@ -516,7 +514,7 @@ int vc_uniscr_check(struct vc_data *vc)
 	p = (unsigned short *)vc->vc_origin;
 	mask = vc->vc_hi_font_mask | 0xff;
 	for (y = 0; y < vc->vc_rows; y++) {
-		char32_t *line = uniscr->lines[y];
+		u32 *line = uniscr->lines[y];
 		for (x = 0; x < vc->vc_cols; x++) {
 			u16 glyph = scr_readw(p++) & mask;
 			line[x] = inverse_translate(vc, glyph, true);
@@ -550,7 +548,7 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 		 */
 		row = (pos - vc->vc_origin) / vc->vc_size_row;
 		col = ((pos - vc->vc_origin) % vc->vc_size_row) / 2;
-		memcpy(dest, &uniscr->lines[row][col], nr * sizeof(char32_t));
+		memcpy(dest, &uniscr->lines[row][col], nr * sizeof(u32));
 	} else {
 		/*
 		 * Scrollback is active. For now let's simply backtranslate
@@ -560,7 +558,7 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 		 */
 		u16 *p = (u16 *)pos;
 		int mask = vc->vc_hi_font_mask | 0xff;
-		char32_t *uni_buf = dest;
+		u32 *uni_buf = dest;
 		while (nr--) {
 			u16 glyph = scr_readw(p++) & mask;
 			*uni_buf++ = inverse_translate(vc, glyph, true);
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/11] tty: vt: remove struct uni_screen
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
                   ` (3 preceding siblings ...)
  2023-01-12  8:01 ` [PATCH 05/11] tty: vt: remove char32_t typedef Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12  9:42   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 07/11] tty: vt: replace BUG_ON() by WARN_ON_ONCE() Jiri Slaby (SUSE)
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

It contains only lines with pointers to characters (u32s). So use
simple clear 'u32 **lines' all over the code.

This avoids zero-length arrays. It also makes the allocation less
error-prone (size of the struct wasn't taken into account at all).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c            | 117 ++++++++++++++++-----------------
 include/linux/console_struct.h |   3 +-
 2 files changed, 59 insertions(+), 61 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 86c18522231b..119b3eafef59 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -320,58 +320,55 @@ void schedule_console_callback(void)
  * Our screen buffer is preceded by an array of line pointers so that
  * scrolling only implies some pointer shuffling.
  */
-struct uni_screen {
-	u32 *lines[0];
-};
 
-static struct uni_screen *vc_uniscr_alloc(unsigned int cols, unsigned int rows)
+static u32 **vc_uniscr_alloc(unsigned int cols, unsigned int rows)
 {
-	struct uni_screen *uniscr;
+	u32 **uni_lines;
 	void *p;
-	unsigned int memsize, i, col_size = cols * sizeof(**uniscr->lines);
+	unsigned int memsize, i, col_size = cols * sizeof(**uni_lines);
 
 	/* allocate everything in one go */
 	memsize = col_size * rows;
-	memsize += rows * sizeof(*uniscr->lines);
-	p = vzalloc(memsize);
-	if (!p)
+	memsize += rows * sizeof(*uni_lines);
+	uni_lines = vzalloc(memsize);
+	if (!uni_lines)
 		return NULL;
 
 	/* initial line pointers */
-	uniscr = p;
-	p = uniscr->lines + rows;
+	p = uni_lines + rows;
 	for (i = 0; i < rows; i++) {
-		uniscr->lines[i] = p;
+		uni_lines[i] = p;
 		p += col_size;
 	}
-	return uniscr;
+
+	return uni_lines;
 }
 
-static void vc_uniscr_free(struct uni_screen *uniscr)
+static void vc_uniscr_free(u32 **uni_lines)
 {
-	vfree(uniscr);
+	vfree(uni_lines);
 }
 
-static void vc_uniscr_set(struct vc_data *vc, struct uni_screen *new_uniscr)
+static void vc_uniscr_set(struct vc_data *vc, u32 **new_uni_lines)
 {
-	vc_uniscr_free(vc->vc_uni_screen);
-	vc->vc_uni_screen = new_uniscr;
+	vc_uniscr_free(vc->vc_uni_lines);
+	vc->vc_uni_lines = new_uni_lines;
 }
 
 static void vc_uniscr_putc(struct vc_data *vc, u32 uc)
 {
-	struct uni_screen *uniscr = vc->vc_uni_screen;
+	u32 **uni_lines = vc->vc_uni_lines;
 
-	if (uniscr)
-		uniscr->lines[vc->state.y][vc->state.x] = uc;
+	if (uni_lines)
+		uni_lines[vc->state.y][vc->state.x] = uc;
 }
 
 static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
 {
-	struct uni_screen *uniscr = vc->vc_uni_screen;
+	u32 **uni_lines = vc->vc_uni_lines;
 
-	if (uniscr) {
-		u32 *ln = uniscr->lines[vc->state.y];
+	if (uni_lines) {
+		u32 *ln = uni_lines[vc->state.y];
 		unsigned int x = vc->state.x, cols = vc->vc_cols;
 
 		memmove(&ln[x + nr], &ln[x], (cols - x - nr) * sizeof(*ln));
@@ -381,10 +378,10 @@ static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
 
 static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
 {
-	struct uni_screen *uniscr = vc->vc_uni_screen;
+	u32 **uni_lines = vc->vc_uni_lines;
 
-	if (uniscr) {
-		u32 *ln = uniscr->lines[vc->state.y];
+	if (uni_lines) {
+		u32 *ln = uni_lines[vc->state.y];
 		unsigned int x = vc->state.x, cols = vc->vc_cols;
 
 		memcpy(&ln[x], &ln[x + nr], (cols - x - nr) * sizeof(*ln));
@@ -395,10 +392,10 @@ static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
 static void vc_uniscr_clear_line(struct vc_data *vc, unsigned int x,
 				 unsigned int nr)
 {
-	struct uni_screen *uniscr = vc->vc_uni_screen;
+	u32 **uni_lines = vc->vc_uni_lines;
 
-	if (uniscr) {
-		u32 *ln = uniscr->lines[vc->state.y];
+	if (uni_lines) {
+		u32 *ln = uni_lines[vc->state.y];
 
 		memset32(&ln[x], ' ', nr);
 	}
@@ -407,22 +404,22 @@ static void vc_uniscr_clear_line(struct vc_data *vc, unsigned int x,
 static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
 				  unsigned int nr)
 {
-	struct uni_screen *uniscr = vc->vc_uni_screen;
+	u32 **uni_lines = vc->vc_uni_lines;
 
-	if (uniscr) {
+	if (uni_lines) {
 		unsigned int cols = vc->vc_cols;
 
 		while (nr--)
-			memset32(uniscr->lines[y++], ' ', cols);
+			memset32(uni_lines[y++], ' ', cols);
 	}
 }
 
 static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
 			     enum con_scroll dir, unsigned int nr)
 {
-	struct uni_screen *uniscr = vc->vc_uni_screen;
+	u32 **uni_lines = vc->vc_uni_lines;
 
-	if (uniscr) {
+	if (uni_lines) {
 		unsigned int i, j, k, sz, d, clear;
 
 		sz = b - t;
@@ -433,7 +430,7 @@ static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
 			d = sz - nr;
 		}
 		for (i = 0; i < gcd(d, sz); i++) {
-			u32 *tmp = uniscr->lines[t + i];
+			u32 *tmp = uni_lines[t + i];
 			j = i;
 			while (1) {
 				k = j + d;
@@ -441,31 +438,31 @@ static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
 					k -= sz;
 				if (k == i)
 					break;
-				uniscr->lines[t + j] = uniscr->lines[t + k];
+				uni_lines[t + j] = uni_lines[t + k];
 				j = k;
 			}
-			uniscr->lines[t + j] = tmp;
+			uni_lines[t + j] = tmp;
 		}
 		vc_uniscr_clear_lines(vc, clear, nr);
 	}
 }
 
-static void vc_uniscr_copy_area(struct uni_screen *dst,
+static void vc_uniscr_copy_area(u32 **dst_lines,
 				unsigned int dst_cols,
 				unsigned int dst_rows,
-				struct uni_screen *src,
+				u32 **src_lines,
 				unsigned int src_cols,
 				unsigned int src_top_row,
 				unsigned int src_bot_row)
 {
 	unsigned int dst_row = 0;
 
-	if (!dst)
+	if (!dst_lines)
 		return;
 
 	while (src_top_row < src_bot_row) {
-		u32 *src_line = src->lines[src_top_row];
-		u32 *dst_line = dst->lines[dst_row];
+		u32 *src_line = src_lines[src_top_row];
+		u32 *dst_line = dst_lines[dst_row];
 
 		memcpy(dst_line, src_line, src_cols * sizeof(*src_line));
 		if (dst_cols - src_cols)
@@ -474,7 +471,7 @@ static void vc_uniscr_copy_area(struct uni_screen *dst,
 		dst_row++;
 	}
 	while (dst_row < dst_rows) {
-		u32 *dst_line = dst->lines[dst_row];
+		u32 *dst_line = dst_lines[dst_row];
 
 		memset32(dst_line, ' ', dst_cols);
 		dst_row++;
@@ -489,7 +486,7 @@ static void vc_uniscr_copy_area(struct uni_screen *dst,
  */
 int vc_uniscr_check(struct vc_data *vc)
 {
-	struct uni_screen *uniscr;
+	u32 **uni_lines;
 	unsigned short *p;
 	int x, y, mask;
 
@@ -498,11 +495,11 @@ int vc_uniscr_check(struct vc_data *vc)
 	if (!vc->vc_utf)
 		return -ENODATA;
 
-	if (vc->vc_uni_screen)
+	if (vc->vc_uni_lines)
 		return 0;
 
-	uniscr = vc_uniscr_alloc(vc->vc_cols, vc->vc_rows);
-	if (!uniscr)
+	uni_lines = vc_uniscr_alloc(vc->vc_cols, vc->vc_rows);
+	if (!uni_lines)
 		return -ENOMEM;
 
 	/*
@@ -514,14 +511,15 @@ int vc_uniscr_check(struct vc_data *vc)
 	p = (unsigned short *)vc->vc_origin;
 	mask = vc->vc_hi_font_mask | 0xff;
 	for (y = 0; y < vc->vc_rows; y++) {
-		u32 *line = uniscr->lines[y];
+		u32 *line = uni_lines[y];
 		for (x = 0; x < vc->vc_cols; x++) {
 			u16 glyph = scr_readw(p++) & mask;
 			line[x] = inverse_translate(vc, glyph, true);
 		}
 	}
 
-	vc->vc_uni_screen = uniscr;
+	vc->vc_uni_lines = uni_lines;
+
 	return 0;
 }
 
@@ -533,11 +531,11 @@ int vc_uniscr_check(struct vc_data *vc)
 void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 			 unsigned int row, unsigned int col, unsigned int nr)
 {
-	struct uni_screen *uniscr = vc->vc_uni_screen;
+	u32 **uni_lines = vc->vc_uni_lines;
 	int offset = row * vc->vc_size_row + col * 2;
 	unsigned long pos;
 
-	BUG_ON(!uniscr);
+	BUG_ON(!uni_lines);
 
 	pos = (unsigned long)screenpos(vc, offset, viewed);
 	if (pos >= vc->vc_origin && pos < vc->vc_scr_end) {
@@ -548,7 +546,7 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 		 */
 		row = (pos - vc->vc_origin) / vc->vc_size_row;
 		col = ((pos - vc->vc_origin) % vc->vc_size_row) / 2;
-		memcpy(dest, &uniscr->lines[row][col], nr * sizeof(u32));
+		memcpy(dest, &uni_lines[row][col], nr * sizeof(u32));
 	} else {
 		/*
 		 * Scrollback is active. For now let's simply backtranslate
@@ -1150,7 +1148,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 	unsigned int new_cols, new_rows, new_row_size, new_screen_size;
 	unsigned int user;
 	unsigned short *oldscreen, *newscreen;
-	struct uni_screen *new_uniscr = NULL;
+	u32 **new_uniscr = NULL;
 
 	WARN_CONSOLE_UNLOCKED();
 
@@ -1194,7 +1192,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 	if (!newscreen)
 		return -ENOMEM;
 
-	if (vc->vc_uni_screen) {
+	if (vc->vc_uni_lines) {
 		new_uniscr = vc_uniscr_alloc(new_cols, new_rows);
 		if (!new_uniscr) {
 			kfree(newscreen);
@@ -1246,7 +1244,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 	end = old_origin + old_row_size * min(old_rows, new_rows);
 
 	vc_uniscr_copy_area(new_uniscr, new_cols, new_rows,
-			    vc->vc_uni_screen, rlth/2, first_copied_row,
+			    vc->vc_uni_lines, rlth/2, first_copied_row,
 			    min(old_rows, new_rows));
 	vc_uniscr_set(vc, new_uniscr);
 
@@ -4688,10 +4686,11 @@ EXPORT_SYMBOL_GPL(screen_glyph);
 
 u32 screen_glyph_unicode(const struct vc_data *vc, int n)
 {
-	struct uni_screen *uniscr = vc->vc_uni_screen;
+	u32 **uni_lines = vc->vc_uni_lines;
+
+	if (uni_lines)
+		return uni_lines[n / vc->vc_cols][n % vc->vc_cols];
 
-	if (uniscr)
-		return uniscr->lines[n / vc->vc_cols][n % vc->vc_cols];
 	return inverse_translate(vc, screen_glyph(vc, n * 2), true);
 }
 EXPORT_SYMBOL_GPL(screen_glyph_unicode);
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index 1518568aaf0f..539f1cd45309 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -18,7 +18,6 @@
 #include <linux/workqueue.h>
 
 struct uni_pagedict;
-struct uni_screen;
 
 #define NPAR 16
 #define VC_TABSTOPS_COUNT	256U
@@ -159,7 +158,7 @@ struct vc_data {
 	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
 	struct uni_pagedict *uni_pagedict;
 	struct uni_pagedict **uni_pagedict_loc; /* [!] Location of uni_pagedict variable for this console */
-	struct uni_screen *vc_uni_screen;	/* unicode screen content */
+	u32 **vc_uni_lines;			/* unicode screen content */
 	/* additional information is in vt_kern.h */
 };
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/11] tty: vt: replace BUG_ON() by WARN_ON_ONCE()
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
                   ` (4 preceding siblings ...)
  2023-01-12  8:01 ` [PATCH 06/11] tty: vt: remove struct uni_screen Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12  9:42   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 08/11] tty: vt: simplify some unicode conditions Jiri Slaby (SUSE)
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

No need to panic in vc_uniscr_copy_line(), just warn. This should never
happen though, as vc_uniscr_check() is supposed to be called before
vc_uniscr_copy_line(). And the former checks vc->vc_uni_lines already.

In any case, use _ONCE as vc_uniscr_copy_line() is called repeatedly for
each line. So don't flood the logs, just in case.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 119b3eafef59..db72375141b0 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -535,7 +535,8 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 	int offset = row * vc->vc_size_row + col * 2;
 	unsigned long pos;
 
-	BUG_ON(!uni_lines);
+	if (WARN_ON_ONCE(!uni_lines))
+		return;
 
 	pos = (unsigned long)screenpos(vc, offset, viewed);
 	if (pos >= vc->vc_origin && pos < vc->vc_scr_end) {
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 08/11] tty: vt: simplify some unicode conditions
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
                   ` (5 preceding siblings ...)
  2023-01-12  8:01 ` [PATCH 07/11] tty: vt: replace BUG_ON() by WARN_ON_ONCE() Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12  9:52   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 09/11] tty: vt: separate array juggling to juggle_array() Jiri Slaby (SUSE)
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

After previous patches, we can simply test vc->vc_uni_lines, so do so in
many unicode functions. This makes the code more compact. And even use
  if (!)
    return;
in vc_uniscr_scroll(), so that the whole code is indented on the left.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 85 +++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 49 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index db72375141b0..74db07b32abe 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -357,18 +357,14 @@ static void vc_uniscr_set(struct vc_data *vc, u32 **new_uni_lines)
 
 static void vc_uniscr_putc(struct vc_data *vc, u32 uc)
 {
-	u32 **uni_lines = vc->vc_uni_lines;
-
-	if (uni_lines)
-		uni_lines[vc->state.y][vc->state.x] = uc;
+	if (vc->vc_uni_lines)
+		vc->vc_uni_lines[vc->state.y][vc->state.x] = uc;
 }
 
 static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
 {
-	u32 **uni_lines = vc->vc_uni_lines;
-
-	if (uni_lines) {
-		u32 *ln = uni_lines[vc->state.y];
+	if (vc->vc_uni_lines) {
+		u32 *ln = vc->vc_uni_lines[vc->state.y];
 		unsigned int x = vc->state.x, cols = vc->vc_cols;
 
 		memmove(&ln[x + nr], &ln[x], (cols - x - nr) * sizeof(*ln));
@@ -378,10 +374,8 @@ static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
 
 static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
 {
-	u32 **uni_lines = vc->vc_uni_lines;
-
-	if (uni_lines) {
-		u32 *ln = uni_lines[vc->state.y];
+	if (vc->vc_uni_lines) {
+		u32 *ln = vc->vc_uni_lines[vc->state.y];
 		unsigned int x = vc->state.x, cols = vc->vc_cols;
 
 		memcpy(&ln[x], &ln[x + nr], (cols - x - nr) * sizeof(*ln));
@@ -392,59 +386,52 @@ static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
 static void vc_uniscr_clear_line(struct vc_data *vc, unsigned int x,
 				 unsigned int nr)
 {
-	u32 **uni_lines = vc->vc_uni_lines;
-
-	if (uni_lines) {
-		u32 *ln = uni_lines[vc->state.y];
-
-		memset32(&ln[x], ' ', nr);
-	}
+	if (vc->vc_uni_lines)
+		memset32(&vc->vc_uni_lines[vc->state.y][x], ' ', nr);
 }
 
 static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
 				  unsigned int nr)
 {
-	u32 **uni_lines = vc->vc_uni_lines;
-
-	if (uni_lines) {
-		unsigned int cols = vc->vc_cols;
-
+	if (vc->vc_uni_lines)
 		while (nr--)
-			memset32(uni_lines[y++], ' ', cols);
-	}
+			memset32(vc->vc_uni_lines[y++], ' ', vc->vc_cols);
 }
 
 static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
 			     enum con_scroll dir, unsigned int nr)
 {
 	u32 **uni_lines = vc->vc_uni_lines;
+	unsigned int i, j, k, sz, d, clear;
 
-	if (uni_lines) {
-		unsigned int i, j, k, sz, d, clear;
+	if (!uni_lines)
+		return;
 
-		sz = b - t;
-		clear = b - nr;
-		d = nr;
-		if (dir == SM_DOWN) {
-			clear = t;
-			d = sz - nr;
-		}
-		for (i = 0; i < gcd(d, sz); i++) {
-			u32 *tmp = uni_lines[t + i];
-			j = i;
-			while (1) {
-				k = j + d;
-				if (k >= sz)
-					k -= sz;
-				if (k == i)
-					break;
-				uni_lines[t + j] = uni_lines[t + k];
-				j = k;
-			}
-			uni_lines[t + j] = tmp;
+	sz = b - t;
+	clear = b - nr;
+	d = nr;
+
+	if (dir == SM_DOWN) {
+		clear = t;
+		d = sz - nr;
+	}
+
+	for (i = 0; i < gcd(d, sz); i++) {
+		u32 *tmp = uni_lines[t + i];
+		j = i;
+		while (1) {
+			k = j + d;
+			if (k >= sz)
+				k -= sz;
+			if (k == i)
+				break;
+			uni_lines[t + j] = uni_lines[t + k];
+			j = k;
 		}
-		vc_uniscr_clear_lines(vc, clear, nr);
+		uni_lines[t + j] = tmp;
 	}
+
+	vc_uniscr_clear_lines(vc, clear, nr);
 }
 
 static void vc_uniscr_copy_area(u32 **dst_lines,
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 09/11] tty: vt: separate array juggling to juggle_array()
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
                   ` (6 preceding siblings ...)
  2023-01-12  8:01 ` [PATCH 08/11] tty: vt: simplify some unicode conditions Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12 10:15   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 10/11] tty: vt: saner names for more scroll variables Jiri Slaby (SUSE)
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

The algorithm used for scrolling is the array juggling. It has
complexity O(N) and space complexity O(1). I.e. quite fast w/o
requirements for temporary storage.

Move the algorithm to a separate function so it is obvious what it is.
It is almost generic (except the array type), so if anyone else wants
array rotation, feel free to make it generic and move it to include/.

And rename all the variables from i, j, k, sz, d, and so on to something
saner.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 52 ++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 74db07b32abe..7cda18b7ee3d 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -398,40 +398,44 @@ static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
 			memset32(vc->vc_uni_lines[y++], ' ', vc->vc_cols);
 }
 
+/* juggling array rotation algorithm (complexity O(N), size complexity O(1)) */
+static void juggle_array(u32 **array, unsigned int size, unsigned int nr)
+{
+	unsigned int gcd_idx;
+
+	for (gcd_idx = 0; gcd_idx < gcd(nr, size); gcd_idx++) {
+		u32 *gcd_idx_val = array[gcd_idx];
+		unsigned int dst_idx = gcd_idx;
+
+		while (1) {
+			unsigned int src_idx = (dst_idx + nr) % size;
+			if (src_idx == gcd_idx)
+				break;
+
+			array[dst_idx] = array[src_idx];
+			dst_idx = src_idx;
+		}
+
+		array[dst_idx] = gcd_idx_val;
+	}
+}
+
 static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
 			     enum con_scroll dir, unsigned int nr)
 {
 	u32 **uni_lines = vc->vc_uni_lines;
-	unsigned int i, j, k, sz, d, clear;
+	unsigned int size = b - t;
 
 	if (!uni_lines)
 		return;
 
-	sz = b - t;
-	clear = b - nr;
-	d = nr;
-
 	if (dir == SM_DOWN) {
-		clear = t;
-		d = sz - nr;
-	}
-
-	for (i = 0; i < gcd(d, sz); i++) {
-		u32 *tmp = uni_lines[t + i];
-		j = i;
-		while (1) {
-			k = j + d;
-			if (k >= sz)
-				k -= sz;
-			if (k == i)
-				break;
-			uni_lines[t + j] = uni_lines[t + k];
-			j = k;
-		}
-		uni_lines[t + j] = tmp;
+		juggle_array(&uni_lines[top], size, size - nr);
+		vc_uniscr_clear_lines(vc, t, nr);
+	} else {
+		juggle_array(&uni_lines[top], size, nr);
+		vc_uniscr_clear_lines(vc, b - nr, nr);
 	}
-
-	vc_uniscr_clear_lines(vc, clear, nr);
 }
 
 static void vc_uniscr_copy_area(u32 **dst_lines,
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 10/11] tty: vt: saner names for more scroll variables
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
                   ` (7 preceding siblings ...)
  2023-01-12  8:01 ` [PATCH 09/11] tty: vt: separate array juggling to juggle_array() Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12  9:59   ` Ilpo Järvinen
  2023-01-12  8:01 ` [PATCH 11/11] tty: vt: cache row count in con_scroll() Jiri Slaby (SUSE)
  2023-01-12  8:43 ` [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Ilpo Järvinen
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

Rename more variables (t, b, s, d) -> (top, bottom, src, dst) to make
them more obvious.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 7cda18b7ee3d..165c81211bdc 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -420,21 +420,22 @@ static void juggle_array(u32 **array, unsigned int size, unsigned int nr)
 	}
 }
 
-static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
-			     enum con_scroll dir, unsigned int nr)
+static void vc_uniscr_scroll(struct vc_data *vc, unsigned int top,
+			     unsigned int bottom, enum con_scroll dir,
+			     unsigned int nr)
 {
 	u32 **uni_lines = vc->vc_uni_lines;
-	unsigned int size = b - t;
+	unsigned int size = bottom - top;
 
 	if (!uni_lines)
 		return;
 
 	if (dir == SM_DOWN) {
 		juggle_array(&uni_lines[top], size, size - nr);
-		vc_uniscr_clear_lines(vc, t, nr);
+		vc_uniscr_clear_lines(vc, top, nr);
 	} else {
 		juggle_array(&uni_lines[top], size, nr);
-		vc_uniscr_clear_lines(vc, b - nr, nr);
+		vc_uniscr_clear_lines(vc, bottom - nr, nr);
 	}
 }
 
@@ -556,27 +557,30 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 	}
 }
 
-static void con_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
-		enum con_scroll dir, unsigned int nr)
+static void con_scroll(struct vc_data *vc, unsigned int top,
+		       unsigned int bottom, enum con_scroll dir,
+		       unsigned int nr)
 {
-	u16 *clear, *d, *s;
+	u16 *clear, *dst, *src;
 
-	if (t + nr >= b)
-		nr = b - t - 1;
-	if (b > vc->vc_rows || t >= b || nr < 1)
+	if (top + nr >= bottom)
+		nr = bottom - top - 1;
+	if (bottom > vc->vc_rows || top >= bottom || nr < 1)
 		return;
-	vc_uniscr_scroll(vc, t, b, dir, nr);
-	if (con_is_visible(vc) && vc->vc_sw->con_scroll(vc, t, b, dir, nr))
+
+	vc_uniscr_scroll(vc, top, bottom, dir, nr);
+	if (con_is_visible(vc) &&
+			vc->vc_sw->con_scroll(vc, top, bottom, dir, nr))
 		return;
 
-	s = clear = (u16 *)(vc->vc_origin + vc->vc_size_row * t);
-	d = (u16 *)(vc->vc_origin + vc->vc_size_row * (t + nr));
+	src = clear = (u16 *)(vc->vc_origin + vc->vc_size_row * top);
+	dst = (u16 *)(vc->vc_origin + vc->vc_size_row * (top + nr));
 
 	if (dir == SM_UP) {
-		clear = s + (b - t - nr) * vc->vc_cols;
-		swap(s, d);
+		clear = src + (bottom - top - nr) * vc->vc_cols;
+		swap(src, dst);
 	}
-	scr_memmovew(d, s, (b - t - nr) * vc->vc_size_row);
+	scr_memmovew(dst, src, (bottom - top - nr) * vc->vc_size_row);
 	scr_memsetw(clear, vc->vc_video_erase_char, vc->vc_size_row * nr);
 }
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 11/11] tty: vt: cache row count in con_scroll()
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
                   ` (8 preceding siblings ...)
  2023-01-12  8:01 ` [PATCH 10/11] tty: vt: saner names for more scroll variables Jiri Slaby (SUSE)
@ 2023-01-12  8:01 ` Jiri Slaby (SUSE)
  2023-01-12 10:00   ` Ilpo Järvinen
  2023-01-12  8:43 ` [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Ilpo Järvinen
  10 siblings, 1 reply; 23+ messages in thread
From: Jiri Slaby (SUSE) @ 2023-01-12  8:01 UTC (permalink / raw)
  To: gregkh; +Cc: Kees Cook, linux-serial, linux-kernel, Jiri Slaby (SUSE)

It's used on few places, so make the code easier to follow by caching
the subtraction result.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/vt/vt.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 165c81211bdc..671304b31f9f 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -561,10 +561,11 @@ static void con_scroll(struct vc_data *vc, unsigned int top,
 		       unsigned int bottom, enum con_scroll dir,
 		       unsigned int nr)
 {
+	unsigned int rows = bottom - top;
 	u16 *clear, *dst, *src;
 
 	if (top + nr >= bottom)
-		nr = bottom - top - 1;
+		nr = rows - 1;
 	if (bottom > vc->vc_rows || top >= bottom || nr < 1)
 		return;
 
@@ -577,10 +578,10 @@ static void con_scroll(struct vc_data *vc, unsigned int top,
 	dst = (u16 *)(vc->vc_origin + vc->vc_size_row * (top + nr));
 
 	if (dir == SM_UP) {
-		clear = src + (bottom - top - nr) * vc->vc_cols;
+		clear = src + (rows - nr) * vc->vc_cols;
 		swap(src, dst);
 	}
-	scr_memmovew(dst, src, (bottom - top - nr) * vc->vc_size_row);
+	scr_memmovew(dst, src, (rows - nr) * vc->vc_size_row);
 	scr_memsetw(clear, vc->vc_video_erase_char, vc->vc_size_row * nr);
 }
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 02/11] tty: vt: drop get_vc_uniscr()
  2023-01-12  8:01 ` [PATCH 02/11] tty: vt: drop get_vc_uniscr() Jiri Slaby (SUSE)
@ 2023-01-12  8:41   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  8:41 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: gregkh, Kees Cook, linux-serial, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> Its definition depends on the NO_VC_UNI_SCREEN macro. But that is never
> defined, so remove all this completely. I.e. expand the macro to
> vc->vc_uni_screen everywhere.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>


-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check()
  2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
                   ` (9 preceding siblings ...)
  2023-01-12  8:01 ` [PATCH 11/11] tty: vt: cache row count in con_scroll() Jiri Slaby (SUSE)
@ 2023-01-12  8:43 ` Ilpo Järvinen
  10 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  8:43 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: Greg Kroah-Hartman, Kees Cook, linux-serial, LKML

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> VC_UNI_SCREEN_DEBUG is always defined as 0, so this code is never
> executed. Drop it along with VC_UNI_SCREEN_DEBUG.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/11] tty: vt: remove reference to undefined NO_VC_UNI_SCREEN
  2023-01-12  8:01 ` [PATCH 03/11] tty: vt: remove reference to undefined NO_VC_UNI_SCREEN Jiri Slaby (SUSE)
@ 2023-01-12  8:44   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  8:44 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: Greg Kroah-Hartman, Kees Cook, linux-serial, LKML

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> NO_VC_UNI_SCREEN is defined nowhere. Remove the last reference to it.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 7e5baf9f8ad8..561c82e120cf 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -495,9 +495,6 @@ int vc_uniscr_check(struct vc_data *vc)
>  	unsigned short *p;
>  	int x, y, mask;
>  
> -	if (__is_defined(NO_VC_UNI_SCREEN))
> -		return -EOPNOTSUPP;
> -
>  	WARN_CONSOLE_UNLOCKED();
>  
>  	if (!vc->vc_utf)
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/11] tty: vt: remove char32_t typedef
  2023-01-12  8:01 ` [PATCH 05/11] tty: vt: remove char32_t typedef Jiri Slaby (SUSE)
@ 2023-01-12  8:52   ` Ilpo Järvinen
  2023-01-12  9:34   ` Ilpo Järvinen
  1 sibling, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  8:52 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: Greg Kroah-Hartman, Kees Cook, linux-serial, LKML

[-- Attachment #1: Type: text/plain, Size: 4552 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> It boils down to uint32_t, so use u32 directly, instead. This makes the
> code more obvious.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 3ae0212f1aa7..86c18522231b 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -316,14 +316,12 @@ void schedule_console_callback(void)
>   * Code to manage unicode-based screen buffers
>   */
>  
> -typedef uint32_t char32_t;
> -
>  /*
>   * Our screen buffer is preceded by an array of line pointers so that
>   * scrolling only implies some pointer shuffling.
>   */
>  struct uni_screen {
> -	char32_t *lines[0];
> +	u32 *lines[0];
>  };
>  
>  static struct uni_screen *vc_uniscr_alloc(unsigned int cols, unsigned int rows)
> @@ -360,7 +358,7 @@ static void vc_uniscr_set(struct vc_data *vc, struct uni_screen *new_uniscr)
>  	vc->vc_uni_screen = new_uniscr;
>  }
>  
> -static void vc_uniscr_putc(struct vc_data *vc, char32_t uc)
> +static void vc_uniscr_putc(struct vc_data *vc, u32 uc)
>  {
>  	struct uni_screen *uniscr = vc->vc_uni_screen;
>  
> @@ -373,7 +371,7 @@ static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
>  	struct uni_screen *uniscr = vc->vc_uni_screen;
>  
>  	if (uniscr) {
> -		char32_t *ln = uniscr->lines[vc->state.y];
> +		u32 *ln = uniscr->lines[vc->state.y];
>  		unsigned int x = vc->state.x, cols = vc->vc_cols;
>  
>  		memmove(&ln[x + nr], &ln[x], (cols - x - nr) * sizeof(*ln));
> @@ -386,7 +384,7 @@ static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
>  	struct uni_screen *uniscr = vc->vc_uni_screen;
>  
>  	if (uniscr) {
> -		char32_t *ln = uniscr->lines[vc->state.y];
> +		u32 *ln = uniscr->lines[vc->state.y];
>  		unsigned int x = vc->state.x, cols = vc->vc_cols;
>  
>  		memcpy(&ln[x], &ln[x + nr], (cols - x - nr) * sizeof(*ln));
> @@ -400,7 +398,7 @@ static void vc_uniscr_clear_line(struct vc_data *vc, unsigned int x,
>  	struct uni_screen *uniscr = vc->vc_uni_screen;
>  
>  	if (uniscr) {
> -		char32_t *ln = uniscr->lines[vc->state.y];
> +		u32 *ln = uniscr->lines[vc->state.y];
>  
>  		memset32(&ln[x], ' ', nr);
>  	}
> @@ -435,7 +433,7 @@ static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
>  			d = sz - nr;
>  		}
>  		for (i = 0; i < gcd(d, sz); i++) {
> -			char32_t *tmp = uniscr->lines[t + i];
> +			u32 *tmp = uniscr->lines[t + i];
>  			j = i;
>  			while (1) {
>  				k = j + d;
> @@ -466,8 +464,8 @@ static void vc_uniscr_copy_area(struct uni_screen *dst,
>  		return;
>  
>  	while (src_top_row < src_bot_row) {
> -		char32_t *src_line = src->lines[src_top_row];
> -		char32_t *dst_line = dst->lines[dst_row];
> +		u32 *src_line = src->lines[src_top_row];
> +		u32 *dst_line = dst->lines[dst_row];
>  
>  		memcpy(dst_line, src_line, src_cols * sizeof(*src_line));
>  		if (dst_cols - src_cols)
> @@ -476,7 +474,7 @@ static void vc_uniscr_copy_area(struct uni_screen *dst,
>  		dst_row++;
>  	}
>  	while (dst_row < dst_rows) {
> -		char32_t *dst_line = dst->lines[dst_row];
> +		u32 *dst_line = dst->lines[dst_row];
>  
>  		memset32(dst_line, ' ', dst_cols);
>  		dst_row++;
> @@ -516,7 +514,7 @@ int vc_uniscr_check(struct vc_data *vc)
>  	p = (unsigned short *)vc->vc_origin;
>  	mask = vc->vc_hi_font_mask | 0xff;
>  	for (y = 0; y < vc->vc_rows; y++) {
> -		char32_t *line = uniscr->lines[y];
> +		u32 *line = uniscr->lines[y];
>  		for (x = 0; x < vc->vc_cols; x++) {
>  			u16 glyph = scr_readw(p++) & mask;
>  			line[x] = inverse_translate(vc, glyph, true);
> @@ -550,7 +548,7 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
>  		 */
>  		row = (pos - vc->vc_origin) / vc->vc_size_row;
>  		col = ((pos - vc->vc_origin) % vc->vc_size_row) / 2;
> -		memcpy(dest, &uniscr->lines[row][col], nr * sizeof(char32_t));
> +		memcpy(dest, &uniscr->lines[row][col], nr * sizeof(u32));
>  	} else {
>  		/*
>  		 * Scrollback is active. For now let's simply backtranslate
> @@ -560,7 +558,7 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
>  		 */
>  		u16 *p = (u16 *)pos;
>  		int mask = vc->vc_hi_font_mask | 0xff;
> -		char32_t *uni_buf = dest;
> +		u32 *uni_buf = dest;
>  		while (nr--) {
>  			u16 glyph = scr_readw(p++) & mask;
>  			*uni_buf++ = inverse_translate(vc, glyph, true);
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 04/11] tty: vt: use sizeof(*variable) where possible
  2023-01-12  8:01 ` [PATCH 04/11] tty: vt: use sizeof(*variable) where possible Jiri Slaby (SUSE)
@ 2023-01-12  8:58   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  8:58 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: Greg Kroah-Hartman, Kees Cook, linux-serial, LKML

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> Instead of sizeof(type), use sizeof(*variable) which is preferred. We
> are going to remove the unicode's char32_t typedef, so this makes the
> switch easier.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 561c82e120cf..3ae0212f1aa7 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -330,11 +330,11 @@ static struct uni_screen *vc_uniscr_alloc(unsigned int cols, unsigned int rows)
>  {
>  	struct uni_screen *uniscr;
>  	void *p;
> -	unsigned int memsize, i;
> +	unsigned int memsize, i, col_size = cols * sizeof(**uniscr->lines);
>  
>  	/* allocate everything in one go */
> -	memsize = cols * rows * sizeof(char32_t);
> -	memsize += rows * sizeof(char32_t *);
> +	memsize = col_size * rows;
> +	memsize += rows * sizeof(*uniscr->lines);

Wouldn't something from linux/overflow.h be more appropriate for these?

-- 
 i.

>  	p = vzalloc(memsize);
>  	if (!p)
>  		return NULL;
> @@ -344,7 +344,7 @@ static struct uni_screen *vc_uniscr_alloc(unsigned int cols, unsigned int rows)
>  	p = uniscr->lines + rows;
>  	for (i = 0; i < rows; i++) {
>  		uniscr->lines[i] = p;
> -		p += cols * sizeof(char32_t);
> +		p += col_size;
>  	}
>  	return uniscr;
>  }
> @@ -469,7 +469,7 @@ static void vc_uniscr_copy_area(struct uni_screen *dst,
>  		char32_t *src_line = src->lines[src_top_row];
>  		char32_t *dst_line = dst->lines[dst_row];
>  
> -		memcpy(dst_line, src_line, src_cols * sizeof(char32_t));
> +		memcpy(dst_line, src_line, src_cols * sizeof(*src_line));
>  		if (dst_cols - src_cols)
>  			memset32(dst_line + src_cols, ' ', dst_cols - src_cols);
>  		src_top_row++;
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/11] tty: vt: remove char32_t typedef
  2023-01-12  8:01 ` [PATCH 05/11] tty: vt: remove char32_t typedef Jiri Slaby (SUSE)
  2023-01-12  8:52   ` Ilpo Järvinen
@ 2023-01-12  9:34   ` Ilpo Järvinen
  1 sibling, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  9:34 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: Greg Kroah-Hartman, Kees Cook, linux-serial, LKML

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> It boils down to uint32_t, so use u32 directly, instead. This makes the
> code more obvious.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 3ae0212f1aa7..86c18522231b 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c

> @@ -550,7 +548,7 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
>  		 */
>  		row = (pos - vc->vc_origin) / vc->vc_size_row;
>  		col = ((pos - vc->vc_origin) % vc->vc_size_row) / 2;
> -		memcpy(dest, &uniscr->lines[row][col], nr * sizeof(char32_t));
> +		memcpy(dest, &uniscr->lines[row][col], nr * sizeof(u32));

Btw, could this be ... * sizeof(**uniscr->lines) instead? It would seem 
slightly safer here.

-- 
 i.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 06/11] tty: vt: remove struct uni_screen
  2023-01-12  8:01 ` [PATCH 06/11] tty: vt: remove struct uni_screen Jiri Slaby (SUSE)
@ 2023-01-12  9:42   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  9:42 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: gregkh, Kees Cook, linux-serial, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> It contains only lines with pointers to characters (u32s). So use
> simple clear 'u32 **lines' all over the code.
> 
> This avoids zero-length arrays. It also makes the allocation less
> error-prone (size of the struct wasn't taken into account at all).
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 07/11] tty: vt: replace BUG_ON() by WARN_ON_ONCE()
  2023-01-12  8:01 ` [PATCH 07/11] tty: vt: replace BUG_ON() by WARN_ON_ONCE() Jiri Slaby (SUSE)
@ 2023-01-12  9:42   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  9:42 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: gregkh, Kees Cook, linux-serial, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1134 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> No need to panic in vc_uniscr_copy_line(), just warn. This should never
> happen though, as vc_uniscr_check() is supposed to be called before
> vc_uniscr_copy_line(). And the former checks vc->vc_uni_lines already.
> 
> In any case, use _ONCE as vc_uniscr_copy_line() is called repeatedly for
> each line. So don't flood the logs, just in case.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 119b3eafef59..db72375141b0 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -535,7 +535,8 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
>  	int offset = row * vc->vc_size_row + col * 2;
>  	unsigned long pos;
>  
> -	BUG_ON(!uni_lines);
> +	if (WARN_ON_ONCE(!uni_lines))
> +		return;
>  
>  	pos = (unsigned long)screenpos(vc, offset, viewed);
>  	if (pos >= vc->vc_origin && pos < vc->vc_scr_end) {
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 08/11] tty: vt: simplify some unicode conditions
  2023-01-12  8:01 ` [PATCH 08/11] tty: vt: simplify some unicode conditions Jiri Slaby (SUSE)
@ 2023-01-12  9:52   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  9:52 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: gregkh, Kees Cook, linux-serial, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4152 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> After previous patches, we can simply test vc->vc_uni_lines, so do so in
> many unicode functions. This makes the code more compact. And even use
>   if (!)
>     return;
> in vc_uniscr_scroll(), so that the whole code is indented on the left.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 85 +++++++++++++++++++--------------------------
>  1 file changed, 36 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index db72375141b0..74db07b32abe 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -357,18 +357,14 @@ static void vc_uniscr_set(struct vc_data *vc, u32 **new_uni_lines)
>  
>  static void vc_uniscr_putc(struct vc_data *vc, u32 uc)
>  {
> -	u32 **uni_lines = vc->vc_uni_lines;
> -
> -	if (uni_lines)
> -		uni_lines[vc->state.y][vc->state.x] = uc;
> +	if (vc->vc_uni_lines)
> +		vc->vc_uni_lines[vc->state.y][vc->state.x] = uc;
>  }
>  
>  static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
>  {
> -	u32 **uni_lines = vc->vc_uni_lines;
> -
> -	if (uni_lines) {
> -		u32 *ln = uni_lines[vc->state.y];
> +	if (vc->vc_uni_lines) {
> +		u32 *ln = vc->vc_uni_lines[vc->state.y];
>  		unsigned int x = vc->state.x, cols = vc->vc_cols;
>  
>  		memmove(&ln[x + nr], &ln[x], (cols - x - nr) * sizeof(*ln));
> @@ -378,10 +374,8 @@ static void vc_uniscr_insert(struct vc_data *vc, unsigned int nr)
>  
>  static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
>  {
> -	u32 **uni_lines = vc->vc_uni_lines;
> -
> -	if (uni_lines) {
> -		u32 *ln = uni_lines[vc->state.y];
> +	if (vc->vc_uni_lines) {
> +		u32 *ln = vc->vc_uni_lines[vc->state.y];
>  		unsigned int x = vc->state.x, cols = vc->vc_cols;
>  
>  		memcpy(&ln[x], &ln[x + nr], (cols - x - nr) * sizeof(*ln));
> @@ -392,59 +386,52 @@ static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr)
>  static void vc_uniscr_clear_line(struct vc_data *vc, unsigned int x,
>  				 unsigned int nr)
>  {
> -	u32 **uni_lines = vc->vc_uni_lines;
> -
> -	if (uni_lines) {
> -		u32 *ln = uni_lines[vc->state.y];
> -
> -		memset32(&ln[x], ' ', nr);
> -	}
> +	if (vc->vc_uni_lines)
> +		memset32(&vc->vc_uni_lines[vc->state.y][x], ' ', nr);
>  }
>  
>  static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
>  				  unsigned int nr)
>  {
> -	u32 **uni_lines = vc->vc_uni_lines;
> -
> -	if (uni_lines) {
> -		unsigned int cols = vc->vc_cols;
> -
> +	if (vc->vc_uni_lines)
>  		while (nr--)
> -			memset32(uni_lines[y++], ' ', cols);
> -	}
> +			memset32(vc->vc_uni_lines[y++], ' ', vc->vc_cols);
>  }
>  
>  static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
>  			     enum con_scroll dir, unsigned int nr)
>  {
>  	u32 **uni_lines = vc->vc_uni_lines;
> +	unsigned int i, j, k, sz, d, clear;
>  
> -	if (uni_lines) {
> -		unsigned int i, j, k, sz, d, clear;
> +	if (!uni_lines)
> +		return;
>  
> -		sz = b - t;
> -		clear = b - nr;
> -		d = nr;
> -		if (dir == SM_DOWN) {
> -			clear = t;
> -			d = sz - nr;
> -		}
> -		for (i = 0; i < gcd(d, sz); i++) {
> -			u32 *tmp = uni_lines[t + i];
> -			j = i;
> -			while (1) {
> -				k = j + d;
> -				if (k >= sz)
> -					k -= sz;
> -				if (k == i)
> -					break;
> -				uni_lines[t + j] = uni_lines[t + k];
> -				j = k;
> -			}
> -			uni_lines[t + j] = tmp;
> +	sz = b - t;
> +	clear = b - nr;
> +	d = nr;
> +
> +	if (dir == SM_DOWN) {
> +		clear = t;
> +		d = sz - nr;
> +	}
> +
> +	for (i = 0; i < gcd(d, sz); i++) {
> +		u32 *tmp = uni_lines[t + i];
> +		j = i;
> +		while (1) {
> +			k = j + d;
> +			if (k >= sz)
> +				k -= sz;
> +			if (k == i)
> +				break;
> +			uni_lines[t + j] = uni_lines[t + k];
> +			j = k;
>  		}
> -		vc_uniscr_clear_lines(vc, clear, nr);
> +		uni_lines[t + j] = tmp;
>  	}
> +
> +	vc_uniscr_clear_lines(vc, clear, nr);
>  }
>  
>  static void vc_uniscr_copy_area(u32 **dst_lines,
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

With the note that increased density of "vc[-_]" per line feels a step 
backwards on readability even if the number of lines is less, IMHO.

-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 10/11] tty: vt: saner names for more scroll variables
  2023-01-12  8:01 ` [PATCH 10/11] tty: vt: saner names for more scroll variables Jiri Slaby (SUSE)
@ 2023-01-12  9:59   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12  9:59 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: Greg Kroah-Hartman, Kees Cook, linux-serial, LKML

[-- Attachment #1: Type: text/plain, Size: 3018 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> Rename more variables (t, b, s, d) -> (top, bottom, src, dst) to make
> them more obvious.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 40 ++++++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 7cda18b7ee3d..165c81211bdc 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -420,21 +420,22 @@ static void juggle_array(u32 **array, unsigned int size, unsigned int nr)
>  	}
>  }
>  
> -static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
> -			     enum con_scroll dir, unsigned int nr)
> +static void vc_uniscr_scroll(struct vc_data *vc, unsigned int top,
> +			     unsigned int bottom, enum con_scroll dir,
> +			     unsigned int nr)
>  {
>  	u32 **uni_lines = vc->vc_uni_lines;
> -	unsigned int size = b - t;
> +	unsigned int size = bottom - top;
>  
>  	if (!uni_lines)
>  		return;
>  
>  	if (dir == SM_DOWN) {
>  		juggle_array(&uni_lines[top], size, size - nr);
> -		vc_uniscr_clear_lines(vc, t, nr);
> +		vc_uniscr_clear_lines(vc, top, nr);
>  	} else {
>  		juggle_array(&uni_lines[top], size, nr);
> -		vc_uniscr_clear_lines(vc, b - nr, nr);
> +		vc_uniscr_clear_lines(vc, bottom - nr, nr);
>  	}
>  }
>  
> @@ -556,27 +557,30 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
>  	}
>  }
>  
> -static void con_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
> -		enum con_scroll dir, unsigned int nr)
> +static void con_scroll(struct vc_data *vc, unsigned int top,
> +		       unsigned int bottom, enum con_scroll dir,
> +		       unsigned int nr)
>  {
> -	u16 *clear, *d, *s;
> +	u16 *clear, *dst, *src;
>  
> -	if (t + nr >= b)
> -		nr = b - t - 1;
> -	if (b > vc->vc_rows || t >= b || nr < 1)
> +	if (top + nr >= bottom)
> +		nr = bottom - top - 1;
> +	if (bottom > vc->vc_rows || top >= bottom || nr < 1)
>  		return;
> -	vc_uniscr_scroll(vc, t, b, dir, nr);
> -	if (con_is_visible(vc) && vc->vc_sw->con_scroll(vc, t, b, dir, nr))
> +
> +	vc_uniscr_scroll(vc, top, bottom, dir, nr);
> +	if (con_is_visible(vc) &&
> +			vc->vc_sw->con_scroll(vc, top, bottom, dir, nr))

I'd keep this on one line.

>  		return;
>  
> -	s = clear = (u16 *)(vc->vc_origin + vc->vc_size_row * t);
> -	d = (u16 *)(vc->vc_origin + vc->vc_size_row * (t + nr));
> +	src = clear = (u16 *)(vc->vc_origin + vc->vc_size_row * top);
> +	dst = (u16 *)(vc->vc_origin + vc->vc_size_row * (top + nr));
>  
>  	if (dir == SM_UP) {
> -		clear = s + (b - t - nr) * vc->vc_cols;
> -		swap(s, d);
> +		clear = src + (bottom - top - nr) * vc->vc_cols;
> +		swap(src, dst);
>  	}
> -	scr_memmovew(d, s, (b - t - nr) * vc->vc_size_row);
> +	scr_memmovew(dst, src, (bottom - top - nr) * vc->vc_size_row);
>  	scr_memsetw(clear, vc->vc_video_erase_char, vc->vc_size_row * nr);
>  }
>  
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 11/11] tty: vt: cache row count in con_scroll()
  2023-01-12  8:01 ` [PATCH 11/11] tty: vt: cache row count in con_scroll() Jiri Slaby (SUSE)
@ 2023-01-12 10:00   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12 10:00 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: gregkh, Kees Cook, linux-serial, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> It's used on few places, so make the code easier to follow by caching
> the subtraction result.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 165c81211bdc..671304b31f9f 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -561,10 +561,11 @@ static void con_scroll(struct vc_data *vc, unsigned int top,
>  		       unsigned int bottom, enum con_scroll dir,
>  		       unsigned int nr)
>  {
> +	unsigned int rows = bottom - top;
>  	u16 *clear, *dst, *src;
>  
>  	if (top + nr >= bottom)
> -		nr = bottom - top - 1;
> +		nr = rows - 1;
>  	if (bottom > vc->vc_rows || top >= bottom || nr < 1)
>  		return;
>  
> @@ -577,10 +578,10 @@ static void con_scroll(struct vc_data *vc, unsigned int top,
>  	dst = (u16 *)(vc->vc_origin + vc->vc_size_row * (top + nr));
>  
>  	if (dir == SM_UP) {
> -		clear = src + (bottom - top - nr) * vc->vc_cols;
> +		clear = src + (rows - nr) * vc->vc_cols;
>  		swap(src, dst);
>  	}
> -	scr_memmovew(dst, src, (bottom - top - nr) * vc->vc_size_row);
> +	scr_memmovew(dst, src, (rows - nr) * vc->vc_size_row);
>  	scr_memsetw(clear, vc->vc_video_erase_char, vc->vc_size_row * nr);
>  }
>  
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 09/11] tty: vt: separate array juggling to juggle_array()
  2023-01-12  8:01 ` [PATCH 09/11] tty: vt: separate array juggling to juggle_array() Jiri Slaby (SUSE)
@ 2023-01-12 10:15   ` Ilpo Järvinen
  0 siblings, 0 replies; 23+ messages in thread
From: Ilpo Järvinen @ 2023-01-12 10:15 UTC (permalink / raw)
  To: Jiri Slaby (SUSE); +Cc: gregkh, Kees Cook, linux-serial, linux-kernel

On Thu, 12 Jan 2023, Jiri Slaby (SUSE) wrote:

> The algorithm used for scrolling is the array juggling. It has
> complexity O(N) and space complexity O(1). I.e. quite fast w/o
> requirements for temporary storage.
> 
> Move the algorithm to a separate function so it is obvious what it is.
> It is almost generic (except the array type), so if anyone else wants
> array rotation, feel free to make it generic and move it to include/.
> 
> And rename all the variables from i, j, k, sz, d, and so on to something
> saner.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/vt/vt.c | 52 ++++++++++++++++++++++++---------------------
>  1 file changed, 28 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 74db07b32abe..7cda18b7ee3d 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -398,40 +398,44 @@ static void vc_uniscr_clear_lines(struct vc_data *vc, unsigned int y,
>  			memset32(vc->vc_uni_lines[y++], ' ', vc->vc_cols);
>  }
>  
> +/* juggling array rotation algorithm (complexity O(N), size complexity O(1)) */
> +static void juggle_array(u32 **array, unsigned int size, unsigned int nr)
> +{
> +	unsigned int gcd_idx;
> +
> +	for (gcd_idx = 0; gcd_idx < gcd(nr, size); gcd_idx++) {
> +		u32 *gcd_idx_val = array[gcd_idx];
> +		unsigned int dst_idx = gcd_idx;
> +
> +		while (1) {
> +			unsigned int src_idx = (dst_idx + nr) % size;
> +			if (src_idx == gcd_idx)
> +				break;
> +
> +			array[dst_idx] = array[src_idx];
> +			dst_idx = src_idx;
> +		}
> +
> +		array[dst_idx] = gcd_idx_val;
> +	}
> +}
> +
>  static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
>  			     enum con_scroll dir, unsigned int nr)
>  {
>  	u32 **uni_lines = vc->vc_uni_lines;
> -	unsigned int i, j, k, sz, d, clear;
> +	unsigned int size = b - t;
>  
>  	if (!uni_lines)
>  		return;
>  
> -	sz = b - t;
> -	clear = b - nr;
> -	d = nr;
> -
>  	if (dir == SM_DOWN) {
> -		clear = t;
> -		d = sz - nr;
> -	}
> -
> -	for (i = 0; i < gcd(d, sz); i++) {
> -		u32 *tmp = uni_lines[t + i];
> -		j = i;
> -		while (1) {
> -			k = j + d;
> -			if (k >= sz)
> -				k -= sz;
> -			if (k == i)
> -				break;
> -			uni_lines[t + j] = uni_lines[t + k];
> -			j = k;
> -		}
> -		uni_lines[t + j] = tmp;
> +		juggle_array(&uni_lines[top], size, size - nr);

top? Should be t I think.

> +		vc_uniscr_clear_lines(vc, t, nr);
> +	} else {
> +		juggle_array(&uni_lines[top], size, nr);

Ditto.

> +		vc_uniscr_clear_lines(vc, b - nr, nr);
>  	}
> -
> -	vc_uniscr_clear_lines(vc, clear, nr);
>  }
>  
>  static void vc_uniscr_copy_area(u32 **dst_lines,
> 



-- 
 i.


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2023-01-12 10:17 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-12  8:01 [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Jiri Slaby (SUSE)
2023-01-12  8:01 ` [PATCH 02/11] tty: vt: drop get_vc_uniscr() Jiri Slaby (SUSE)
2023-01-12  8:41   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 03/11] tty: vt: remove reference to undefined NO_VC_UNI_SCREEN Jiri Slaby (SUSE)
2023-01-12  8:44   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 04/11] tty: vt: use sizeof(*variable) where possible Jiri Slaby (SUSE)
2023-01-12  8:58   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 05/11] tty: vt: remove char32_t typedef Jiri Slaby (SUSE)
2023-01-12  8:52   ` Ilpo Järvinen
2023-01-12  9:34   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 06/11] tty: vt: remove struct uni_screen Jiri Slaby (SUSE)
2023-01-12  9:42   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 07/11] tty: vt: replace BUG_ON() by WARN_ON_ONCE() Jiri Slaby (SUSE)
2023-01-12  9:42   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 08/11] tty: vt: simplify some unicode conditions Jiri Slaby (SUSE)
2023-01-12  9:52   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 09/11] tty: vt: separate array juggling to juggle_array() Jiri Slaby (SUSE)
2023-01-12 10:15   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 10/11] tty: vt: saner names for more scroll variables Jiri Slaby (SUSE)
2023-01-12  9:59   ` Ilpo Järvinen
2023-01-12  8:01 ` [PATCH 11/11] tty: vt: cache row count in con_scroll() Jiri Slaby (SUSE)
2023-01-12 10:00   ` Ilpo Järvinen
2023-01-12  8:43 ` [PATCH 01/11] tty: vt: remove vc_uniscr_debug_check() Ilpo Järvinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).