[PATCH 1/7] tty/vt: consolemap: use ARRAY_SIZE(), part II.

[PATCH 1/7] tty/vt: consolemap: use ARRAY_SIZE(), part II.

[Jiri Slaby]

The code still uses constants (macros) as bounds in loops after commit
17945d317a52 (tty/vt: consolemap: use ARRAY_SIZE()). The contants are at
least macros used also in the definition of the arrays. But use
ARRAY_SIZE() on two more places to ensure the loops never run out of
bounds even if the array definition change.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index fff97ae87e00..8aa7a48b3647 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -232,7 +232,7 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
 	}
 	memset(q, 0, MAX_GLYPH);
 
-	for (j = 0; j < E_TABSZ; j++) {
+	for (j = 0; j < ARRAY_SIZE(translations[i]); j++) {
 		glyph = conv_uni_to_pc(conp, t[j]);
 		if (glyph >= 0 && glyph < MAX_GLYPH && q[glyph] < 32) {
 			/* prefer '-' above SHY etc. */
@@ -367,7 +367,7 @@ int con_get_trans_old(unsigned char __user * arg)
 	unsigned char outbuf[E_TABSZ];
 
 	console_lock();
-	for (i = 0; i < E_TABSZ ; i++)
+	for (i = 0; i < ARRAY_SIZE(outbuf); i++)
 	{
 		ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
 		outbuf[i] = (ch & ~0xff) ? 0 : ch;
-- 
2.36.1

[PATCH 2/7] tty/vt: consolemap: remove unused parameter from set_inverse_trans_unicode()

[Jiri Slaby]

conp is unused in set_inverse_trans_unicode(), remove it.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 8aa7a48b3647..a7507c5c3154 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -241,8 +241,7 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
 	}
 }
 
-static void set_inverse_trans_unicode(struct vc_data *conp,
-				      struct uni_pagedict *p)
+static void set_inverse_trans_unicode(struct uni_pagedict *p)
 {
 	unsigned int d, r, g;
 	u16 *inv;
@@ -327,7 +326,7 @@ static void update_user_maps(void)
 		p = *vc_cons[i].d->vc_uni_pagedir_loc;
 		if (p && p != q) {
 			set_inverse_transl(vc_cons[i].d, p, USER_MAP);
-			set_inverse_trans_unicode(vc_cons[i].d, p);
+			set_inverse_trans_unicode(p);
 			q = p;
 		}
 	}
@@ -678,7 +677,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
 
 	for (enum translation_map m = FIRST_MAP; m <= LAST_MAP; m++)
 		set_inverse_transl(vc, dict, m);
-	set_inverse_trans_unicode(vc, dict);
+	set_inverse_trans_unicode(dict);
 
 out_unlock:
 	console_unlock();
@@ -741,7 +740,7 @@ int con_set_default_unimap(struct vc_data *vc)
 
 	for (enum translation_map m = FIRST_MAP; m <= LAST_MAP; m++)
 		set_inverse_transl(vc, dict, m);
-	set_inverse_trans_unicode(vc, dict);
+	set_inverse_trans_unicode(dict);
 	dflt = dict;
 	return err;
 }
-- 
2.36.1

[PATCH 3/7] tty/vt: consolemap: saner variable names in set_inverse_trans_unicode()

[Jiri Slaby]

The function still uses too vague parameter name after commit
50c92a1b2d50 (tty/vt: consolemap: saner variable names in
set_inverse_trans_unicode()).

So use "dict" instead of "p" for that parameter too.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index a7507c5c3154..a69dfda8e3d0 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -241,17 +241,17 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
 	}
 }
 
-static void set_inverse_trans_unicode(struct uni_pagedict *p)
+static void set_inverse_trans_unicode(struct uni_pagedict *dict)
 {
 	unsigned int d, r, g;
 	u16 *inv;
 
-	if (!p)
+	if (!dict)
 		return;
 
-	inv = p->inverse_trans_unicode;
+	inv = dict->inverse_trans_unicode;
 	if (!inv) {
-		inv = p->inverse_trans_unicode = kmalloc_array(MAX_GLYPH,
+		inv = dict->inverse_trans_unicode = kmalloc_array(MAX_GLYPH,
 				sizeof(*inv), GFP_KERNEL);
 		if (!inv)
 			return;
@@ -259,7 +259,7 @@ static void set_inverse_trans_unicode(struct uni_pagedict *p)
 	memset(inv, 0, MAX_GLYPH * sizeof(*inv));
 
 	for (d = 0; d < UNI_DIRS; d++) {
-		u16 **dir = p->uni_pgdir[d];
+		u16 **dir = dict->uni_pgdir[d];
 		if (!dir)
 			continue;
 		for (r = 0; r < UNI_DIR_ROWS; r++) {
-- 
2.36.1

[PATCH 4/7] tty/vt: consolemap: saner variable names in set_inverse_transl()

[Jiri Slaby]

The function uses too vague variable names like i, j, k for iterators, p,
q, p1, p2 for pointers etc.

Rename all these, so that it is clear what is going on:
- dict: for dictionaries.
- d, r, g: for dir, row, glyph iterators -- these are unsigned now.
- dir, row: for directory and row pointers.
- glyph: for the glyph.
- and so on...

This is a lot of shuffling, but the result pays off, IMO.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index a69dfda8e3d0..3d0e10dac6d9 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -214,29 +214,29 @@ struct uni_pagedict {
 
 static struct uni_pagedict *dflt;
 
-static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
+static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *dict,
 	       enum translation_map m)
 {
-	int j, glyph;
 	unsigned short *t = translations[m];
-	unsigned char *q;
+	unsigned char *inv;
 
-	if (!p)
+	if (!dict)
 		return;
-	q = p->inverse_translations[m];
+	inv = dict->inverse_translations[m];
 
-	if (!q) {
-		q = p->inverse_translations[m] = kmalloc(MAX_GLYPH, GFP_KERNEL);
-		if (!q)
+	if (!inv) {
+		inv = dict->inverse_translations[m] = kmalloc(MAX_GLYPH,
+				GFP_KERNEL);
+		if (!inv)
 			return;
 	}
-	memset(q, 0, MAX_GLYPH);
+	memset(inv, 0, MAX_GLYPH);
 
-	for (j = 0; j < ARRAY_SIZE(translations[i]); j++) {
-		glyph = conv_uni_to_pc(conp, t[j]);
-		if (glyph >= 0 && glyph < MAX_GLYPH && q[glyph] < 32) {
+	for (unsigned int ch = 0; ch < ARRAY_SIZE(translations[m]); ch++) {
+		int glyph = conv_uni_to_pc(conp, t[ch]);
+		if (glyph >= 0 && glyph < MAX_GLYPH && inv[glyph] < 32) {
 			/* prefer '-' above SHY etc. */
-			q[glyph] = j;
+			inv[glyph] = ch;
 		}
 	}
 }
-- 
2.36.1

Re: [PATCH 4/7] tty/vt: consolemap: saner variable names in set_inverse_transl()

[Ilpo Järvinen]

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

On Tue, 14 Jun 2022, Jiri Slaby wrote:

> The function uses too vague variable names like i, j, k for iterators, p,
> q, p1, p2 for pointers etc.
> 
> Rename all these, so that it is clear what is going on:
> - dict: for dictionaries.
> - d, r, g: for dir, row, glyph iterators -- these are unsigned now.
> - dir, row: for directory and row pointers.
> - glyph: for the glyph.
> - and so on...
> 
> This is a lot of shuffling, but the result pays off, IMO.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  drivers/tty/vt/consolemap.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
> index a69dfda8e3d0..3d0e10dac6d9 100644
> --- a/drivers/tty/vt/consolemap.c
> +++ b/drivers/tty/vt/consolemap.c
> @@ -214,29 +214,29 @@ struct uni_pagedict {
>  
>  static struct uni_pagedict *dflt;
>  
> -static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
> +static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *dict,
>  	       enum translation_map m)
>  {
> -	int j, glyph;
>  	unsigned short *t = translations[m];
> -	unsigned char *q;
> +	unsigned char *inv;
>  
> -	if (!p)
> +	if (!dict)
>  		return;
> -	q = p->inverse_translations[m];
> +	inv = dict->inverse_translations[m];
>  
> -	if (!q) {
> -		q = p->inverse_translations[m] = kmalloc(MAX_GLYPH, GFP_KERNEL);
> -		if (!q)
> +	if (!inv) {
> +		inv = dict->inverse_translations[m] = kmalloc(MAX_GLYPH,
> +				GFP_KERNEL);
> +		if (!inv)
>  			return;
>  	}
> -	memset(q, 0, MAX_GLYPH);
> +	memset(inv, 0, MAX_GLYPH);
>  
> -	for (j = 0; j < ARRAY_SIZE(translations[i]); j++) {
> -		glyph = conv_uni_to_pc(conp, t[j]);
> -		if (glyph >= 0 && glyph < MAX_GLYPH && q[glyph] < 32) {
> +	for (unsigned int ch = 0; ch < ARRAY_SIZE(translations[m]); ch++) {

This removes the compile error you introduced earlier. Other than that:

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

> +		int glyph = conv_uni_to_pc(conp, t[ch]);
> +		if (glyph >= 0 && glyph < MAX_GLYPH && inv[glyph] < 32) {
>  			/* prefer '-' above SHY etc. */
> -			q[glyph] = j;
> +			inv[glyph] = ch;
>  		}
>  	}
>  }
> 

-- 
 i.

Re: [PATCH 4/7] tty/vt: consolemap: saner variable names in set_inverse_transl()

[Jiri Slaby]

On 14. 06. 22, 10:35, Ilpo Järvinen wrote:
>> +	for (unsigned int ch = 0; ch < ARRAY_SIZE(translations[m]); ch++) {
> 
> This removes the compile error you introduced earlier. Other than that:

Bah, let me fix that up. It's an error coming from a rebase -- I rebased 
this on the current tty-next as this was part of v2 of the previous 
series which Greg already applied 8-).

thanks,
-- 
js
suse labs

[PATCH 5/7] tty/vt: consolemap: rename struct vc_data::vc_uni_pagedir*

[Jiri Slaby]

As a follow-up to the commit 4173f018aae1 (tty/vt: consolemap: rename
and document struct uni_pagedir), rename also the members of struct
vc_data. I.e. pagedir -> pagedict. And while touching all the places,
remove also the unnecessary vc_ prefix.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c             | 46 ++++++++++++-------------
 drivers/tty/vt/vt.c                     |  8 ++---
 drivers/usb/misc/sisusbvga/sisusb_con.c |  2 +-
 drivers/video/console/vgacon.c          |  8 ++---
 drivers/video/fbdev/core/fbcon.c        |  8 ++---
 include/linux/console_struct.h          |  4 +--
 6 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 3d0e10dac6d9..16d0d8f04f0e 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -296,7 +296,7 @@ u16 inverse_translate(const struct vc_data *conp, u16 glyph, bool use_unicode)
 	if (glyph >= MAX_GLYPH)
 		return 0;
 
-	p = *conp->vc_uni_pagedir_loc;
+	p = *conp->uni_pagedict_loc;
 	if (!p)
 		return glyph;
 
@@ -323,7 +323,7 @@ static void update_user_maps(void)
 	for (i = 0; i < MAX_NR_CONSOLES; i++) {
 		if (!vc_cons_allocated(i))
 			continue;
-		p = *vc_cons[i].d->vc_uni_pagedir_loc;
+		p = *vc_cons[i].d->uni_pagedict_loc;
 		if (p && p != q) {
 			set_inverse_transl(vc_cons[i].d, p, USER_MAP);
 			set_inverse_trans_unicode(p);
@@ -445,10 +445,10 @@ void con_free_unimap(struct vc_data *vc)
 {
 	struct uni_pagedict *p;
 
-	p = *vc->vc_uni_pagedir_loc;
+	p = *vc->uni_pagedict_loc;
 	if (!p)
 		return;
-	*vc->vc_uni_pagedir_loc = NULL;
+	*vc->uni_pagedict_loc = NULL;
 	if (--p->refcount)
 		return;
 	con_release_unimap(p);
@@ -463,7 +463,7 @@ static int con_unify_unimap(struct vc_data *conp, struct uni_pagedict *dict1)
 	for (cons = 0; cons < MAX_NR_CONSOLES; cons++) {
 		if (!vc_cons_allocated(cons))
 			continue;
-		dict2 = *vc_cons[cons].d->vc_uni_pagedir_loc;
+		dict2 = *vc_cons[cons].d->uni_pagedict_loc;
 		if (!dict2 || dict2 == dict1 || dict2->sum != dict1->sum)
 			continue;
 		for (d = 0; d < UNI_DIRS; d++) {
@@ -487,7 +487,7 @@ static int con_unify_unimap(struct vc_data *conp, struct uni_pagedict *dict1)
 		}
 		if (d == UNI_DIRS) {
 			dict2->refcount++;
-			*conp->vc_uni_pagedir_loc = dict2;
+			*conp->uni_pagedict_loc = dict2;
 			con_release_unimap(dict1);
 			kfree(dict1);
 			return 1;
@@ -531,14 +531,14 @@ con_insert_unipair(struct uni_pagedict *p, u_short unicode, u_short fontpos)
 
 static int con_allocate_new(struct vc_data *vc)
 {
-	struct uni_pagedict *new, *old = *vc->vc_uni_pagedir_loc;
+	struct uni_pagedict *new, *old = *vc->uni_pagedict_loc;
 
 	new = kzalloc(sizeof(*new), GFP_KERNEL);
 	if (!new)
 		return -ENOMEM;
 
 	new->refcount = 1;
-	*vc->vc_uni_pagedir_loc = new;
+	*vc->uni_pagedict_loc = new;
 
 	if (old)
 		old->refcount--;
@@ -549,7 +549,7 @@ static int con_allocate_new(struct vc_data *vc)
 /* Caller must hold the lock */
 static int con_do_clear_unimap(struct vc_data *vc)
 {
-	struct uni_pagedict *old = *vc->vc_uni_pagedir_loc;
+	struct uni_pagedict *old = *vc->uni_pagedict_loc;
 
 	if (!old || old->refcount > 1)
 		return con_allocate_new(vc);
@@ -583,7 +583,7 @@ static struct uni_pagedict *con_unshare_unimap(struct vc_data *vc,
 	if (ret)
 		return ERR_PTR(ret);
 
-	new = *vc->vc_uni_pagedir_loc;
+	new = *vc->uni_pagedict_loc;
 
 	/*
 	 * uni_pgdir is a 32*32*64 table with rows allocated when its first
@@ -616,7 +616,7 @@ static struct uni_pagedict *con_unshare_unimap(struct vc_data *vc,
 				ret = con_insert_unipair(new, uni, row[g]);
 				if (ret) {
 					old->refcount++;
-					*vc->vc_uni_pagedir_loc = old;
+					*vc->uni_pagedict_loc = old;
 					con_release_unimap(new);
 					kfree(new);
 					return ERR_PTR(ret);
@@ -644,7 +644,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
 	console_lock();
 
 	/* Save original vc_unipagdir_loc in case we allocate a new one */
-	dict = *vc->vc_uni_pagedir_loc;
+	dict = *vc->uni_pagedict_loc;
 	if (!dict) {
 		err = -EINVAL;
 		goto out_unlock;
@@ -704,12 +704,12 @@ int con_set_default_unimap(struct vc_data *vc)
 	u16 *dfont;
 
 	if (dflt) {
-		dict = *vc->vc_uni_pagedir_loc;
+		dict = *vc->uni_pagedict_loc;
 		if (dict == dflt)
 			return 0;
 
 		dflt->refcount++;
-		*vc->vc_uni_pagedir_loc = dflt;
+		*vc->uni_pagedict_loc = dflt;
 		if (dict && !--dict->refcount) {
 			con_release_unimap(dict);
 			kfree(dict);
@@ -723,7 +723,7 @@ int con_set_default_unimap(struct vc_data *vc)
 	if (err)
 		return err;
 
-	dict = *vc->vc_uni_pagedir_loc;
+	dict = *vc->uni_pagedict_loc;
 	dfont = dfont_unitable;
 
 	for (fontpos = 0; fontpos < 256U; fontpos++)
@@ -734,7 +734,7 @@ int con_set_default_unimap(struct vc_data *vc)
 		}
 
 	if (con_unify_unimap(vc, dict)) {
-		dflt = *vc->vc_uni_pagedir_loc;
+		dflt = *vc->uni_pagedict_loc;
 		return err;
 	}
 
@@ -757,14 +757,14 @@ int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc)
 {
 	struct uni_pagedict *src;
 
-	if (!*src_vc->vc_uni_pagedir_loc)
+	if (!*src_vc->uni_pagedict_loc)
 		return -EINVAL;
-	if (*dst_vc->vc_uni_pagedir_loc == *src_vc->vc_uni_pagedir_loc)
+	if (*dst_vc->uni_pagedict_loc == *src_vc->uni_pagedict_loc)
 		return 0;
 	con_free_unimap(dst_vc);
-	src = *src_vc->vc_uni_pagedir_loc;
+	src = *src_vc->uni_pagedict_loc;
 	src->refcount++;
-	*dst_vc->vc_uni_pagedir_loc = src;
+	*dst_vc->uni_pagedict_loc = src;
 	return 0;
 }
 EXPORT_SYMBOL(con_copy_unimap);
@@ -791,7 +791,7 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct,
 	console_lock();
 
 	ect = 0;
-	dict = *vc->vc_uni_pagedir_loc;
+	dict = *vc->uni_pagedict_loc;
 	if (!dict)
 		goto unlock;
 
@@ -873,7 +873,7 @@ int conv_uni_to_pc(struct vc_data *conp, long ucs)
 	else if ((ucs & ~UNI_DIRECT_MASK) == UNI_DIRECT_BASE)
 		return ucs & UNI_DIRECT_MASK;
 
-	dict = *conp->vc_uni_pagedir_loc;
+	dict = *conp->uni_pagedict_loc;
 	if (!dict)
 		return -3;
 
@@ -903,7 +903,7 @@ console_map_init(void)
 	int i;
 
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
-		if (vc_cons_allocated(i) && !*vc_cons[i].d->vc_uni_pagedir_loc)
+		if (vc_cons_allocated(i) && !*vc_cons[i].d->uni_pagedict_loc)
 			con_set_default_unimap(vc_cons[i].d);
 }
 
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index c718b0d01e3d..1899b8a5d73e 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1063,10 +1063,10 @@ static void visual_init(struct vc_data *vc, int num, int init)
 	__module_get(vc->vc_sw->owner);
 	vc->vc_num = num;
 	vc->vc_display_fg = &master_display_fg;
-	if (vc->vc_uni_pagedir_loc)
+	if (vc->uni_pagedict_loc)
 		con_free_unimap(vc);
-	vc->vc_uni_pagedir_loc = &vc->vc_uni_pagedir;
-	vc->vc_uni_pagedir = NULL;
+	vc->uni_pagedict_loc = &vc->uni_pagedict;
+	vc->uni_pagedict = NULL;
 	vc->vc_hi_font_mask = 0;
 	vc->vc_complement_mask = 0;
 	vc->vc_can_do_color = 0;
@@ -1136,7 +1136,7 @@ int vc_allocate(unsigned int currcons)	/* return 0 on success */
 
 	visual_init(vc, currcons, 1);
 
-	if (!*vc->vc_uni_pagedir_loc)
+	if (!*vc->uni_pagedict_loc)
 		con_set_default_unimap(vc);
 
 	err = -EINVAL;
diff --git a/drivers/usb/misc/sisusbvga/sisusb_con.c b/drivers/usb/misc/sisusbvga/sisusb_con.c
index dfa0d5ce6012..fcb95fb639e0 100644
--- a/drivers/usb/misc/sisusbvga/sisusb_con.c
+++ b/drivers/usb/misc/sisusbvga/sisusb_con.c
@@ -248,7 +248,7 @@ sisusbcon_init(struct vc_data *c, int init)
 	 */
 	kref_get(&sisusb->kref);
 
-	if (!*c->vc_uni_pagedir_loc)
+	if (!*c->uni_pagedict_loc)
 		con_set_default_unimap(c);
 
 	mutex_unlock(&sisusb->lock);
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 058a78b8dbcf..fcdf017e2665 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -367,10 +367,10 @@ static void vgacon_init(struct vc_data *c, int init)
 	c->vc_complement_mask = 0x7700;
 	if (vga_512_chars)
 		c->vc_hi_font_mask = 0x0800;
-	p = *c->vc_uni_pagedir_loc;
-	if (c->vc_uni_pagedir_loc != &vgacon_uni_pagedir) {
+	p = *c->uni_pagedict_loc;
+	if (c->uni_pagedict_loc != &vgacon_uni_pagedir) {
 		con_free_unimap(c);
-		c->vc_uni_pagedir_loc = &vgacon_uni_pagedir;
+		c->uni_pagedict_loc = &vgacon_uni_pagedir;
 		vgacon_refcount++;
 	}
 	if (!vgacon_uni_pagedir && p)
@@ -392,7 +392,7 @@ static void vgacon_deinit(struct vc_data *c)
 
 	if (!--vgacon_refcount)
 		con_free_unimap(c);
-	c->vc_uni_pagedir_loc = &c->vc_uni_pagedir;
+	c->uni_pagedict_loc = &c->uni_pagedict;
 	con_set_default_unimap(c);
 }
 
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 1be8aa9f8074..238a136c0e11 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1060,9 +1060,9 @@ static void fbcon_init(struct vc_data *vc, int init)
 			vc->vc_complement_mask <<= 1;
 	}
 
-	if (!*svc->vc_uni_pagedir_loc)
+	if (!*svc->uni_pagedict_loc)
 		con_set_default_unimap(svc);
-	if (!*vc->vc_uni_pagedir_loc)
+	if (!*vc->uni_pagedict_loc)
 		con_copy_unimap(vc, svc);
 
 	ops = info->fbcon_par;
@@ -1384,9 +1384,9 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
 			vc->vc_complement_mask <<= 1;
 	}
 
-	if (!*svc->vc_uni_pagedir_loc)
+	if (!*svc->uni_pagedict_loc)
 		con_set_default_unimap(svc);
-	if (!*vc->vc_uni_pagedir_loc)
+	if (!*vc->uni_pagedict_loc)
 		con_copy_unimap(vc, svc);
 
 	cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres);
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index f75033f0277f..1518568aaf0f 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -157,8 +157,8 @@ struct vc_data {
 	unsigned int	vc_bell_duration;	/* Console bell duration */
 	unsigned short	vc_cur_blink_ms;	/* Cursor blink duration */
 	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
-	struct uni_pagedict *vc_uni_pagedir;
-	struct uni_pagedict **vc_uni_pagedir_loc; /* [!] Location of uni_pagedict variable for this console */
+	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 */
 	/* additional information is in vt_kern.h */
 };
-- 
2.36.1

Re: [PATCH 5/7] tty/vt: consolemap: rename struct vc_data::vc_uni_pagedir*

[Ilpo Järvinen]

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

On Tue, 14 Jun 2022, Jiri Slaby wrote:

> As a follow-up to the commit 4173f018aae1 (tty/vt: consolemap: rename
> and document struct uni_pagedir), rename also the members of struct
> vc_data. I.e. pagedir -> pagedict. And while touching all the places,
> remove also the unnecessary vc_ prefix.
> 
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

Look fine,

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


You might want to rename vgacon_uni_pagedir too. :-)


-- 
 i.

[PATCH 6/7] tty/vt: consolemap: improve UNI_*() macros definitions

[Jiri Slaby]

Use FIELD_GET() and GENMASK() helpers instead of direct shifts and ANDs.
This makes the code even more obvious. I didn't know about the helpers
at the time of writing the macros.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 16d0d8f04f0e..9e94ec0e0f83 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -23,6 +23,8 @@
  * stack overflow.
  */
 
+#include <linux/bitfield.h>
+#include <linux/bits.h>
 #include <linux/module.h>
 #include <linux/kd.h>
 #include <linux/errno.h>
@@ -190,10 +192,17 @@ static enum translation_map inv_translate[MAX_NR_CONSOLES];
 #define UNI_DIR_ROWS	32U
 #define UNI_ROW_GLYPHS	64U
 
-#define UNI_DIR(uni)		( (uni)                   >> 11)
-#define UNI_ROW(uni)		(((uni) & GENMASK(10, 6)) >>  6)
-#define UNI_GLYPH(uni)		( (uni) & GENMASK( 5, 0))
-#define UNI(dir, row, glyph)	(((dir) << 11) | ((row) << 6) | (glyph))
+#define UNI_DIR_BITS		GENMASK(15, 11)
+#define UNI_ROW_BITS		GENMASK(10,  6)
+#define UNI_GLYPH_BITS		GENMASK( 5,  0)
+
+#define UNI_DIR(uni)		FIELD_GET(UNI_DIR_BITS, (uni))
+#define UNI_ROW(uni)		FIELD_GET(UNI_ROW_BITS, (uni))
+#define UNI_GLYPH(uni)		FIELD_GET(UNI_GLYPH_BITS, (uni))
+
+#define UNI(dir, row, glyph)	(FIELD_PREP(UNI_DIR_BITS, (dir)) | \
+				 FIELD_PREP(UNI_ROW_BITS, (row)) | \
+				 FIELD_PREP(UNI_GLYPH_BITS, (glyph)))
 
 /**
  * struct uni_pagedict -- unicode directory
-- 
2.36.1

[PATCH 7/7] tty/vt: consolemap: remove dflt reset from con_do_clear_unimap()

[Jiri Slaby]

con_do_clear_unimap() sets dflt to NULL and then calls
con_release_unimap() which does the very same as the first thing. So
remove the former as it is apparently superfluous.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 9e94ec0e0f83..5f3e58165b98 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -563,8 +563,6 @@ static int con_do_clear_unimap(struct vc_data *vc)
 	if (!old || old->refcount > 1)
 		return con_allocate_new(vc);
 
-	if (old == dflt)
-		dflt = NULL;
 	old->sum = 0;
 	con_release_unimap(old);
 
-- 
2.36.1

Re: [PATCH 7/7] tty/vt: consolemap: remove dflt reset from con_do_clear_unimap()

[Ilpo Järvinen]

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

On Tue, 14 Jun 2022, Jiri Slaby wrote:

> con_do_clear_unimap() sets dflt to NULL and then calls
> con_release_unimap() which does the very same as the first thing. So
> remove the former as it is apparently superfluous.
> 
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

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

...to all patches in this series that weren't individually replied to.

-- 
 i.

Re: [PATCH 1/7] tty/vt: consolemap: use ARRAY_SIZE(), part II.

[Ilpo Järvinen]

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

On Tue, 14 Jun 2022, Jiri Slaby wrote:

> The code still uses constants (macros) as bounds in loops after commit
> 17945d317a52 (tty/vt: consolemap: use ARRAY_SIZE()). The contants are at
> least macros used also in the definition of the arrays. But use
> ARRAY_SIZE() on two more places to ensure the loops never run out of
> bounds even if the array definition change.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  drivers/tty/vt/consolemap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
> index fff97ae87e00..8aa7a48b3647 100644
> --- a/drivers/tty/vt/consolemap.c
> +++ b/drivers/tty/vt/consolemap.c
> @@ -232,7 +232,7 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
>  	}
>  	memset(q, 0, MAX_GLYPH);
>  
> -	for (j = 0; j < E_TABSZ; j++) {
> +	for (j = 0; j < ARRAY_SIZE(translations[i]); j++) {

Any particular reason why you left its definition to have 256 instead of 
E_TABSZ (even after the patch series I mean):

static unsigned short translations[][256] = {


>  		glyph = conv_uni_to_pc(conp, t[j]);
>  		if (glyph >= 0 && glyph < MAX_GLYPH && q[glyph] < 32) {
>  			/* prefer '-' above SHY etc. */
> @@ -367,7 +367,7 @@ int con_get_trans_old(unsigned char __user * arg)
>  	unsigned char outbuf[E_TABSZ];
>  
>  	console_lock();
> -	for (i = 0; i < E_TABSZ ; i++)
> +	for (i = 0; i < ARRAY_SIZE(outbuf); i++)
>  	{
>  		ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
>  		outbuf[i] = (ch & ~0xff) ? 0 : ch;
> 

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

-- 
 i.

Re: [PATCH 1/7] tty/vt: consolemap: use ARRAY_SIZE(), part II.

[Ilpo Järvinen]

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

Hi all,

Err, I take my Reviewed-by back. I just found an issue with this commit 
while reviewing a later patch in the series that undos that error.

On Tue, 14 Jun 2022, Ilpo Järvinen wrote:

> On Tue, 14 Jun 2022, Jiri Slaby wrote:
> 
> > The code still uses constants (macros) as bounds in loops after commit
> > 17945d317a52 (tty/vt: consolemap: use ARRAY_SIZE()). The contants are at
> > least macros used also in the definition of the arrays. But use
> > ARRAY_SIZE() on two more places to ensure the loops never run out of
> > bounds even if the array definition change.
> > 
> > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > ---
> >  drivers/tty/vt/consolemap.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
> > index fff97ae87e00..8aa7a48b3647 100644
> > --- a/drivers/tty/vt/consolemap.c
> > +++ b/drivers/tty/vt/consolemap.c
> > @@ -232,7 +232,7 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
> >  	}
> >  	memset(q, 0, MAX_GLYPH);
> >  
> > -	for (j = 0; j < E_TABSZ; j++) {
> > +	for (j = 0; j < ARRAY_SIZE(translations[i]); j++) {

There's no i variable in this function.

> Any particular reason why you left its definition to have 256 instead of 
> E_TABSZ (even after the patch series I mean):
> 
> static unsigned short translations[][256] = {
> 
> 
> >  		glyph = conv_uni_to_pc(conp, t[j]);
> >  		if (glyph >= 0 && glyph < MAX_GLYPH && q[glyph] < 32) {
> >  			/* prefer '-' above SHY etc. */
> > @@ -367,7 +367,7 @@ int con_get_trans_old(unsigned char __user * arg)
> >  	unsigned char outbuf[E_TABSZ];
> >  
> >  	console_lock();
> > -	for (i = 0; i < E_TABSZ ; i++)
> > +	for (i = 0; i < ARRAY_SIZE(outbuf); i++)
> >  	{
> >  		ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
> >  		outbuf[i] = (ch & ~0xff) ? 0 : ch;
> > 
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

This patch is not fine so I take my rev-by back.


-- 
 i.

Re: [PATCH 1/7] tty/vt: consolemap: use ARRAY_SIZE(), part II.

[Jiri Slaby]

On 14. 06. 22, 10:17, Ilpo Järvinen wrote:
> On Tue, 14 Jun 2022, Jiri Slaby wrote:
> 
>> The code still uses constants (macros) as bounds in loops after commit
>> 17945d317a52 (tty/vt: consolemap: use ARRAY_SIZE()). The contants are at
>> least macros used also in the definition of the arrays. But use
>> ARRAY_SIZE() on two more places to ensure the loops never run out of
>> bounds even if the array definition change.
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> ---
>>   drivers/tty/vt/consolemap.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
>> index fff97ae87e00..8aa7a48b3647 100644
>> --- a/drivers/tty/vt/consolemap.c
>> +++ b/drivers/tty/vt/consolemap.c
>> @@ -232,7 +232,7 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
>>   	}
>>   	memset(q, 0, MAX_GLYPH);
>>   
>> -	for (j = 0; j < E_TABSZ; j++) {
>> +	for (j = 0; j < ARRAY_SIZE(translations[i]); j++) {
> 
> Any particular reason why you left its definition to have 256 instead of
> E_TABSZ (even after the patch series I mean):
> 
> static unsigned short translations[][256] = {

I will. (Only if it wasn't so badly chosen name. And even exported to 
userspace.)

thanks,
-- 
js
suse labs

[PATCH v2 1/8] tty/vt: consolemap: use ARRAY_SIZE(), part II.

[Jiri Slaby]

The code still uses constants (macros) as bounds in loops after commit
17945d317a52 (tty/vt: consolemap: use ARRAY_SIZE()). The contants are at
least macros used also in the definition of the arrays. But use
ARRAY_SIZE() on two more places to ensure the loops never run out of
bounds even if the array definition change.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---

Notes:
    [v2]
    - fix build error (which was fixed by 4/7 in v1)

 drivers/tty/vt/consolemap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index fff97ae87e00..2039237b5266 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -232,7 +232,7 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
 	}
 	memset(q, 0, MAX_GLYPH);
 
-	for (j = 0; j < E_TABSZ; j++) {
+	for (j = 0; j < ARRAY_SIZE(translations[m]); j++) {
 		glyph = conv_uni_to_pc(conp, t[j]);
 		if (glyph >= 0 && glyph < MAX_GLYPH && q[glyph] < 32) {
 			/* prefer '-' above SHY etc. */
@@ -367,7 +367,7 @@ int con_get_trans_old(unsigned char __user * arg)
 	unsigned char outbuf[E_TABSZ];
 
 	console_lock();
-	for (i = 0; i < E_TABSZ ; i++)
+	for (i = 0; i < ARRAY_SIZE(outbuf); i++)
 	{
 		ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
 		outbuf[i] = (ch & ~0xff) ? 0 : ch;
-- 
2.36.1

[PATCH v2 2/8] tty/vt: consolemap: remove unused parameter from set_inverse_trans_unicode()

[Jiri Slaby]

conp is unused in set_inverse_trans_unicode(), remove it.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 2039237b5266..c5f5fa39d7b2 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -241,8 +241,7 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
 	}
 }
 
-static void set_inverse_trans_unicode(struct vc_data *conp,
-				      struct uni_pagedict *p)
+static void set_inverse_trans_unicode(struct uni_pagedict *p)
 {
 	unsigned int d, r, g;
 	u16 *inv;
@@ -327,7 +326,7 @@ static void update_user_maps(void)
 		p = *vc_cons[i].d->vc_uni_pagedir_loc;
 		if (p && p != q) {
 			set_inverse_transl(vc_cons[i].d, p, USER_MAP);
-			set_inverse_trans_unicode(vc_cons[i].d, p);
+			set_inverse_trans_unicode(p);
 			q = p;
 		}
 	}
@@ -678,7 +677,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
 
 	for (enum translation_map m = FIRST_MAP; m <= LAST_MAP; m++)
 		set_inverse_transl(vc, dict, m);
-	set_inverse_trans_unicode(vc, dict);
+	set_inverse_trans_unicode(dict);
 
 out_unlock:
 	console_unlock();
@@ -741,7 +740,7 @@ int con_set_default_unimap(struct vc_data *vc)
 
 	for (enum translation_map m = FIRST_MAP; m <= LAST_MAP; m++)
 		set_inverse_transl(vc, dict, m);
-	set_inverse_trans_unicode(vc, dict);
+	set_inverse_trans_unicode(dict);
 	dflt = dict;
 	return err;
 }
-- 
2.36.1

[PATCH v2 3/8] tty/vt: consolemap: saner variable names in set_inverse_trans_unicode()

[Jiri Slaby]

The function still uses too vague parameter name after commit
50c92a1b2d50 (tty/vt: consolemap: saner variable names in
set_inverse_trans_unicode()).

So use "dict" instead of "p" for that parameter too.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index c5f5fa39d7b2..55fb466361c1 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -241,17 +241,17 @@ static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
 	}
 }
 
-static void set_inverse_trans_unicode(struct uni_pagedict *p)
+static void set_inverse_trans_unicode(struct uni_pagedict *dict)
 {
 	unsigned int d, r, g;
 	u16 *inv;
 
-	if (!p)
+	if (!dict)
 		return;
 
-	inv = p->inverse_trans_unicode;
+	inv = dict->inverse_trans_unicode;
 	if (!inv) {
-		inv = p->inverse_trans_unicode = kmalloc_array(MAX_GLYPH,
+		inv = dict->inverse_trans_unicode = kmalloc_array(MAX_GLYPH,
 				sizeof(*inv), GFP_KERNEL);
 		if (!inv)
 			return;
@@ -259,7 +259,7 @@ static void set_inverse_trans_unicode(struct uni_pagedict *p)
 	memset(inv, 0, MAX_GLYPH * sizeof(*inv));
 
 	for (d = 0; d < UNI_DIRS; d++) {
-		u16 **dir = p->uni_pgdir[d];
+		u16 **dir = dict->uni_pgdir[d];
 		if (!dir)
 			continue;
 		for (r = 0; r < UNI_DIR_ROWS; r++) {
-- 
2.36.1

[PATCH v2 4/8] tty/vt: consolemap: saner variable names in set_inverse_transl()

[Jiri Slaby]

The function uses too vague variable names like i, j, k for iterators, p,
q, p1, p2 for pointers etc.

Rename all these, so that it is clear what is going on:
- dict: for dictionaries.
- d, r, g: for dir, row, glyph iterators -- these are unsigned now.
- dir, row: for directory and row pointers.
- glyph: for the glyph.
- and so on...

This is a lot of shuffling, but the result pays off, IMO.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 55fb466361c1..3d0e10dac6d9 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -214,29 +214,29 @@ struct uni_pagedict {
 
 static struct uni_pagedict *dflt;
 
-static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *p,
+static void set_inverse_transl(struct vc_data *conp, struct uni_pagedict *dict,
 	       enum translation_map m)
 {
-	int j, glyph;
 	unsigned short *t = translations[m];
-	unsigned char *q;
+	unsigned char *inv;
 
-	if (!p)
+	if (!dict)
 		return;
-	q = p->inverse_translations[m];
+	inv = dict->inverse_translations[m];
 
-	if (!q) {
-		q = p->inverse_translations[m] = kmalloc(MAX_GLYPH, GFP_KERNEL);
-		if (!q)
+	if (!inv) {
+		inv = dict->inverse_translations[m] = kmalloc(MAX_GLYPH,
+				GFP_KERNEL);
+		if (!inv)
 			return;
 	}
-	memset(q, 0, MAX_GLYPH);
+	memset(inv, 0, MAX_GLYPH);
 
-	for (j = 0; j < ARRAY_SIZE(translations[m]); j++) {
-		glyph = conv_uni_to_pc(conp, t[j]);
-		if (glyph >= 0 && glyph < MAX_GLYPH && q[glyph] < 32) {
+	for (unsigned int ch = 0; ch < ARRAY_SIZE(translations[m]); ch++) {
+		int glyph = conv_uni_to_pc(conp, t[ch]);
+		if (glyph >= 0 && glyph < MAX_GLYPH && inv[glyph] < 32) {
 			/* prefer '-' above SHY etc. */
-			q[glyph] = j;
+			inv[glyph] = ch;
 		}
 	}
 }
-- 
2.36.1

[PATCH v2 5/8] tty/vt: consolemap: rename struct vc_data::vc_uni_pagedir*

[Jiri Slaby]

As a follow-up to the commit 4173f018aae1 (tty/vt: consolemap: rename
and document struct uni_pagedir), rename also the members of struct
vc_data. I.e. pagedir -> pagedict. And while touching all the places,
remove also the unnecessary vc_ prefix.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c             | 46 ++++++++++++-------------
 drivers/tty/vt/vt.c                     |  8 ++---
 drivers/usb/misc/sisusbvga/sisusb_con.c |  2 +-
 drivers/video/console/vgacon.c          |  8 ++---
 drivers/video/fbdev/core/fbcon.c        |  8 ++---
 include/linux/console_struct.h          |  4 +--
 6 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 3d0e10dac6d9..16d0d8f04f0e 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -296,7 +296,7 @@ u16 inverse_translate(const struct vc_data *conp, u16 glyph, bool use_unicode)
 	if (glyph >= MAX_GLYPH)
 		return 0;
 
-	p = *conp->vc_uni_pagedir_loc;
+	p = *conp->uni_pagedict_loc;
 	if (!p)
 		return glyph;
 
@@ -323,7 +323,7 @@ static void update_user_maps(void)
 	for (i = 0; i < MAX_NR_CONSOLES; i++) {
 		if (!vc_cons_allocated(i))
 			continue;
-		p = *vc_cons[i].d->vc_uni_pagedir_loc;
+		p = *vc_cons[i].d->uni_pagedict_loc;
 		if (p && p != q) {
 			set_inverse_transl(vc_cons[i].d, p, USER_MAP);
 			set_inverse_trans_unicode(p);
@@ -445,10 +445,10 @@ void con_free_unimap(struct vc_data *vc)
 {
 	struct uni_pagedict *p;
 
-	p = *vc->vc_uni_pagedir_loc;
+	p = *vc->uni_pagedict_loc;
 	if (!p)
 		return;
-	*vc->vc_uni_pagedir_loc = NULL;
+	*vc->uni_pagedict_loc = NULL;
 	if (--p->refcount)
 		return;
 	con_release_unimap(p);
@@ -463,7 +463,7 @@ static int con_unify_unimap(struct vc_data *conp, struct uni_pagedict *dict1)
 	for (cons = 0; cons < MAX_NR_CONSOLES; cons++) {
 		if (!vc_cons_allocated(cons))
 			continue;
-		dict2 = *vc_cons[cons].d->vc_uni_pagedir_loc;
+		dict2 = *vc_cons[cons].d->uni_pagedict_loc;
 		if (!dict2 || dict2 == dict1 || dict2->sum != dict1->sum)
 			continue;
 		for (d = 0; d < UNI_DIRS; d++) {
@@ -487,7 +487,7 @@ static int con_unify_unimap(struct vc_data *conp, struct uni_pagedict *dict1)
 		}
 		if (d == UNI_DIRS) {
 			dict2->refcount++;
-			*conp->vc_uni_pagedir_loc = dict2;
+			*conp->uni_pagedict_loc = dict2;
 			con_release_unimap(dict1);
 			kfree(dict1);
 			return 1;
@@ -531,14 +531,14 @@ con_insert_unipair(struct uni_pagedict *p, u_short unicode, u_short fontpos)
 
 static int con_allocate_new(struct vc_data *vc)
 {
-	struct uni_pagedict *new, *old = *vc->vc_uni_pagedir_loc;
+	struct uni_pagedict *new, *old = *vc->uni_pagedict_loc;
 
 	new = kzalloc(sizeof(*new), GFP_KERNEL);
 	if (!new)
 		return -ENOMEM;
 
 	new->refcount = 1;
-	*vc->vc_uni_pagedir_loc = new;
+	*vc->uni_pagedict_loc = new;
 
 	if (old)
 		old->refcount--;
@@ -549,7 +549,7 @@ static int con_allocate_new(struct vc_data *vc)
 /* Caller must hold the lock */
 static int con_do_clear_unimap(struct vc_data *vc)
 {
-	struct uni_pagedict *old = *vc->vc_uni_pagedir_loc;
+	struct uni_pagedict *old = *vc->uni_pagedict_loc;
 
 	if (!old || old->refcount > 1)
 		return con_allocate_new(vc);
@@ -583,7 +583,7 @@ static struct uni_pagedict *con_unshare_unimap(struct vc_data *vc,
 	if (ret)
 		return ERR_PTR(ret);
 
-	new = *vc->vc_uni_pagedir_loc;
+	new = *vc->uni_pagedict_loc;
 
 	/*
 	 * uni_pgdir is a 32*32*64 table with rows allocated when its first
@@ -616,7 +616,7 @@ static struct uni_pagedict *con_unshare_unimap(struct vc_data *vc,
 				ret = con_insert_unipair(new, uni, row[g]);
 				if (ret) {
 					old->refcount++;
-					*vc->vc_uni_pagedir_loc = old;
+					*vc->uni_pagedict_loc = old;
 					con_release_unimap(new);
 					kfree(new);
 					return ERR_PTR(ret);
@@ -644,7 +644,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
 	console_lock();
 
 	/* Save original vc_unipagdir_loc in case we allocate a new one */
-	dict = *vc->vc_uni_pagedir_loc;
+	dict = *vc->uni_pagedict_loc;
 	if (!dict) {
 		err = -EINVAL;
 		goto out_unlock;
@@ -704,12 +704,12 @@ int con_set_default_unimap(struct vc_data *vc)
 	u16 *dfont;
 
 	if (dflt) {
-		dict = *vc->vc_uni_pagedir_loc;
+		dict = *vc->uni_pagedict_loc;
 		if (dict == dflt)
 			return 0;
 
 		dflt->refcount++;
-		*vc->vc_uni_pagedir_loc = dflt;
+		*vc->uni_pagedict_loc = dflt;
 		if (dict && !--dict->refcount) {
 			con_release_unimap(dict);
 			kfree(dict);
@@ -723,7 +723,7 @@ int con_set_default_unimap(struct vc_data *vc)
 	if (err)
 		return err;
 
-	dict = *vc->vc_uni_pagedir_loc;
+	dict = *vc->uni_pagedict_loc;
 	dfont = dfont_unitable;
 
 	for (fontpos = 0; fontpos < 256U; fontpos++)
@@ -734,7 +734,7 @@ int con_set_default_unimap(struct vc_data *vc)
 		}
 
 	if (con_unify_unimap(vc, dict)) {
-		dflt = *vc->vc_uni_pagedir_loc;
+		dflt = *vc->uni_pagedict_loc;
 		return err;
 	}
 
@@ -757,14 +757,14 @@ int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc)
 {
 	struct uni_pagedict *src;
 
-	if (!*src_vc->vc_uni_pagedir_loc)
+	if (!*src_vc->uni_pagedict_loc)
 		return -EINVAL;
-	if (*dst_vc->vc_uni_pagedir_loc == *src_vc->vc_uni_pagedir_loc)
+	if (*dst_vc->uni_pagedict_loc == *src_vc->uni_pagedict_loc)
 		return 0;
 	con_free_unimap(dst_vc);
-	src = *src_vc->vc_uni_pagedir_loc;
+	src = *src_vc->uni_pagedict_loc;
 	src->refcount++;
-	*dst_vc->vc_uni_pagedir_loc = src;
+	*dst_vc->uni_pagedict_loc = src;
 	return 0;
 }
 EXPORT_SYMBOL(con_copy_unimap);
@@ -791,7 +791,7 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct,
 	console_lock();
 
 	ect = 0;
-	dict = *vc->vc_uni_pagedir_loc;
+	dict = *vc->uni_pagedict_loc;
 	if (!dict)
 		goto unlock;
 
@@ -873,7 +873,7 @@ int conv_uni_to_pc(struct vc_data *conp, long ucs)
 	else if ((ucs & ~UNI_DIRECT_MASK) == UNI_DIRECT_BASE)
 		return ucs & UNI_DIRECT_MASK;
 
-	dict = *conp->vc_uni_pagedir_loc;
+	dict = *conp->uni_pagedict_loc;
 	if (!dict)
 		return -3;
 
@@ -903,7 +903,7 @@ console_map_init(void)
 	int i;
 
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
-		if (vc_cons_allocated(i) && !*vc_cons[i].d->vc_uni_pagedir_loc)
+		if (vc_cons_allocated(i) && !*vc_cons[i].d->uni_pagedict_loc)
 			con_set_default_unimap(vc_cons[i].d);
 }
 
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index c718b0d01e3d..1899b8a5d73e 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1063,10 +1063,10 @@ static void visual_init(struct vc_data *vc, int num, int init)
 	__module_get(vc->vc_sw->owner);
 	vc->vc_num = num;
 	vc->vc_display_fg = &master_display_fg;
-	if (vc->vc_uni_pagedir_loc)
+	if (vc->uni_pagedict_loc)
 		con_free_unimap(vc);
-	vc->vc_uni_pagedir_loc = &vc->vc_uni_pagedir;
-	vc->vc_uni_pagedir = NULL;
+	vc->uni_pagedict_loc = &vc->uni_pagedict;
+	vc->uni_pagedict = NULL;
 	vc->vc_hi_font_mask = 0;
 	vc->vc_complement_mask = 0;
 	vc->vc_can_do_color = 0;
@@ -1136,7 +1136,7 @@ int vc_allocate(unsigned int currcons)	/* return 0 on success */
 
 	visual_init(vc, currcons, 1);
 
-	if (!*vc->vc_uni_pagedir_loc)
+	if (!*vc->uni_pagedict_loc)
 		con_set_default_unimap(vc);
 
 	err = -EINVAL;
diff --git a/drivers/usb/misc/sisusbvga/sisusb_con.c b/drivers/usb/misc/sisusbvga/sisusb_con.c
index dfa0d5ce6012..fcb95fb639e0 100644
--- a/drivers/usb/misc/sisusbvga/sisusb_con.c
+++ b/drivers/usb/misc/sisusbvga/sisusb_con.c
@@ -248,7 +248,7 @@ sisusbcon_init(struct vc_data *c, int init)
 	 */
 	kref_get(&sisusb->kref);
 
-	if (!*c->vc_uni_pagedir_loc)
+	if (!*c->uni_pagedict_loc)
 		con_set_default_unimap(c);
 
 	mutex_unlock(&sisusb->lock);
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 058a78b8dbcf..fcdf017e2665 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -367,10 +367,10 @@ static void vgacon_init(struct vc_data *c, int init)
 	c->vc_complement_mask = 0x7700;
 	if (vga_512_chars)
 		c->vc_hi_font_mask = 0x0800;
-	p = *c->vc_uni_pagedir_loc;
-	if (c->vc_uni_pagedir_loc != &vgacon_uni_pagedir) {
+	p = *c->uni_pagedict_loc;
+	if (c->uni_pagedict_loc != &vgacon_uni_pagedir) {
 		con_free_unimap(c);
-		c->vc_uni_pagedir_loc = &vgacon_uni_pagedir;
+		c->uni_pagedict_loc = &vgacon_uni_pagedir;
 		vgacon_refcount++;
 	}
 	if (!vgacon_uni_pagedir && p)
@@ -392,7 +392,7 @@ static void vgacon_deinit(struct vc_data *c)
 
 	if (!--vgacon_refcount)
 		con_free_unimap(c);
-	c->vc_uni_pagedir_loc = &c->vc_uni_pagedir;
+	c->uni_pagedict_loc = &c->uni_pagedict;
 	con_set_default_unimap(c);
 }
 
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 1be8aa9f8074..238a136c0e11 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1060,9 +1060,9 @@ static void fbcon_init(struct vc_data *vc, int init)
 			vc->vc_complement_mask <<= 1;
 	}
 
-	if (!*svc->vc_uni_pagedir_loc)
+	if (!*svc->uni_pagedict_loc)
 		con_set_default_unimap(svc);
-	if (!*vc->vc_uni_pagedir_loc)
+	if (!*vc->uni_pagedict_loc)
 		con_copy_unimap(vc, svc);
 
 	ops = info->fbcon_par;
@@ -1384,9 +1384,9 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
 			vc->vc_complement_mask <<= 1;
 	}
 
-	if (!*svc->vc_uni_pagedir_loc)
+	if (!*svc->uni_pagedict_loc)
 		con_set_default_unimap(svc);
-	if (!*vc->vc_uni_pagedir_loc)
+	if (!*vc->uni_pagedict_loc)
 		con_copy_unimap(vc, svc);
 
 	cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres);
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index f75033f0277f..1518568aaf0f 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -157,8 +157,8 @@ struct vc_data {
 	unsigned int	vc_bell_duration;	/* Console bell duration */
 	unsigned short	vc_cur_blink_ms;	/* Cursor blink duration */
 	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
-	struct uni_pagedict *vc_uni_pagedir;
-	struct uni_pagedict **vc_uni_pagedir_loc; /* [!] Location of uni_pagedict variable for this console */
+	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 */
 	/* additional information is in vt_kern.h */
 };
-- 
2.36.1

[PATCH v2 6/8] tty/vt: consolemap: improve UNI_*() macros definitions

[Jiri Slaby]

Use FIELD_GET() and GENMASK() helpers instead of direct shifts and ANDs.
This makes the code even more obvious. I didn't know about the helpers
at the time of writing the macros.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 16d0d8f04f0e..9e94ec0e0f83 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -23,6 +23,8 @@
  * stack overflow.
  */
 
+#include <linux/bitfield.h>
+#include <linux/bits.h>
 #include <linux/module.h>
 #include <linux/kd.h>
 #include <linux/errno.h>
@@ -190,10 +192,17 @@ static enum translation_map inv_translate[MAX_NR_CONSOLES];
 #define UNI_DIR_ROWS	32U
 #define UNI_ROW_GLYPHS	64U
 
-#define UNI_DIR(uni)		( (uni)                   >> 11)
-#define UNI_ROW(uni)		(((uni) & GENMASK(10, 6)) >>  6)
-#define UNI_GLYPH(uni)		( (uni) & GENMASK( 5, 0))
-#define UNI(dir, row, glyph)	(((dir) << 11) | ((row) << 6) | (glyph))
+#define UNI_DIR_BITS		GENMASK(15, 11)
+#define UNI_ROW_BITS		GENMASK(10,  6)
+#define UNI_GLYPH_BITS		GENMASK( 5,  0)
+
+#define UNI_DIR(uni)		FIELD_GET(UNI_DIR_BITS, (uni))
+#define UNI_ROW(uni)		FIELD_GET(UNI_ROW_BITS, (uni))
+#define UNI_GLYPH(uni)		FIELD_GET(UNI_GLYPH_BITS, (uni))
+
+#define UNI(dir, row, glyph)	(FIELD_PREP(UNI_DIR_BITS, (dir)) | \
+				 FIELD_PREP(UNI_ROW_BITS, (row)) | \
+				 FIELD_PREP(UNI_GLYPH_BITS, (glyph)))
 
 /**
  * struct uni_pagedict -- unicode directory
-- 
2.36.1

[PATCH v2 7/8] tty/vt: consolemap: remove dflt reset from con_do_clear_unimap()

[Jiri Slaby]

con_do_clear_unimap() sets dflt to NULL and then calls
con_release_unimap() which does the very same as the first thing. So
remove the former as it is apparently superfluous.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 9e94ec0e0f83..5f3e58165b98 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -563,8 +563,6 @@ static int con_do_clear_unimap(struct vc_data *vc)
 	if (!old || old->refcount > 1)
 		return con_allocate_new(vc);
 
-	if (old == dflt)
-		dflt = NULL;
 	old->sum = 0;
 	con_release_unimap(old);
 
-- 
2.36.1

[PATCH v2 8/8] tty/vt: consolemap: use E_TABSZ for the translations size

[Jiri Slaby]

The code expects "translations" to have 256 (E_TABSZ) values. Use the
macro instead of the constant to be explicit about this.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---

Notes:
    [v2]
    - this is new in this series

 drivers/tty/vt/consolemap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 5f3e58165b98..f02d21e2a96e 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -38,7 +38,7 @@
 #include <linux/vt_kern.h>
 #include <linux/string.h>
 
-static unsigned short translations[][256] = {
+static unsigned short translations[][E_TABSZ] = {
   /* 8-bit Latin-1 mapped to Unicode -- trivial mapping */
   [LAT1_MAP] = {
     0x0000, 0x0001, 0x0002, 0x0003, 0x0004, 0x0005, 0x0006, 0x0007,
-- 
2.36.1