Linux console tools development discussion
 help / color / mirror / Atom feed
* [kbd] [PATCH] src/libkeymap: add support for parsing more unicode values
@ 2021-02-27 14:36 Anisse Astier
  2021-03-01 14:09 ` Alexey Gladkov
  0 siblings, 1 reply; 6+ messages in thread
From: Anisse Astier @ 2021-02-27 14:36 UTC (permalink / raw)
  To: kbd; +Cc: Alexey Gladkov

The auto-generated (with ckbcomp) file fr-bepo_afnor did not load (even
partially), because of an U+1f12f (copyleft symbol) that is wrongly
parsed, generating this error message:

	too many (160) entries on one line

Fix libkeymap so that the keymap can be parsed, even if the offending
character won't be loaded because of the ushort limitation of the
kb_value KDSKBENT uapi.

It's better to have the keymap partially loaded than not at all.

Signed-off-by: Anisse Astier <anisse@astier.eu>
Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=968195
---
 src/libkeymap/analyze.l | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libkeymap/analyze.l b/src/libkeymap/analyze.l
index 9e76eae..4bdffb4 100644
--- a/src/libkeymap/analyze.l
+++ b/src/libkeymap/analyze.l
@@ -319,7 +319,7 @@ Include			include[ \t]*
 Decimal			[1-9][0-9]*
 Octal			0[0-7]*
 Hex			0[xX][0-9a-fA-F]+
-Unicode			U\+([0-9a-fA-F]){4}
+Unicode			U\+([0-9a-fA-F]){4,6}
 Literal			[a-zA-Z][a-zA-Z_0-9]*
 Octa			([0-7]){1,3}
 Charset			charset|Charset|CharSet|CHARSET
@@ -404,7 +404,7 @@ To                      to|To|TO
 				if (parse_int(yyextra, yytext, yytext + 1, 16, &(yylval->num)) < 0)
 					return(ERROR);
 
-				if (yylval->num >= 0xf000) {
+				if (yylval->num >= 0x10ffff) {
 					ERR(yyextra, _("unicode keysym out of range: %s"),
 						yytext);
 					return(ERROR);
-- 
2.29.2



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

* Re: [kbd] [PATCH] src/libkeymap: add support for parsing more unicode values
  2021-02-27 14:36 [kbd] [PATCH] src/libkeymap: add support for parsing more unicode values Anisse Astier
@ 2021-03-01 14:09 ` Alexey Gladkov
  2021-03-01 14:49   ` Anisse Astier
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Gladkov @ 2021-03-01 14:09 UTC (permalink / raw)
  To: Anisse Astier; +Cc: kbd

On Sat, Feb 27, 2021 at 03:36:11PM +0100, Anisse Astier wrote:
> The auto-generated (with ckbcomp) file fr-bepo_afnor did not load (even
> partially), because of an U+1f12f (copyleft symbol) that is wrongly
> parsed, generating this error message:
> 
> 	too many (160) entries on one line
> 
> Fix libkeymap so that the keymap can be parsed, even if the offending
> character won't be loaded because of the ushort limitation of the
> kb_value KDSKBENT uapi.
> 
> It's better to have the keymap partially loaded than not at all.

Nop. Partially keymap loading is very dangerous. You can get a completely
unusable console. The libkeymap shouldn't break the console if it is known
in advance that the keymap is not correct. You should fix ckbcomp so that
it generates the correct keymap.

> Signed-off-by: Anisse Astier <anisse@astier.eu>
> Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=968195
> ---
>  src/libkeymap/analyze.l | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libkeymap/analyze.l b/src/libkeymap/analyze.l
> index 9e76eae..4bdffb4 100644
> --- a/src/libkeymap/analyze.l
> +++ b/src/libkeymap/analyze.l
> @@ -319,7 +319,7 @@ Include			include[ \t]*
>  Decimal			[1-9][0-9]*
>  Octal			0[0-7]*
>  Hex			0[xX][0-9a-fA-F]+
> -Unicode			U\+([0-9a-fA-F]){4}
> +Unicode			U\+([0-9a-fA-F]){4,6}
>  Literal			[a-zA-Z][a-zA-Z_0-9]*
>  Octa			([0-7]){1,3}
>  Charset			charset|Charset|CharSet|CHARSET
> @@ -404,7 +404,7 @@ To                      to|To|TO
>  				if (parse_int(yyextra, yytext, yytext + 1, 16, &(yylval->num)) < 0)
>  					return(ERROR);
>  
> -				if (yylval->num >= 0xf000) {
> +				if (yylval->num >= 0x10ffff) {
>  					ERR(yyextra, _("unicode keysym out of range: %s"),
>  						yytext);
>  					return(ERROR);
> -- 
> 2.29.2
> 

-- 
Rgrds, legion



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

* Re: [kbd] [PATCH] src/libkeymap: add support for parsing more unicode values
  2021-03-01 14:09 ` Alexey Gladkov
@ 2021-03-01 14:49   ` Anisse Astier
  2021-03-02  0:47     ` Alexey Gladkov
  0 siblings, 1 reply; 6+ messages in thread
From: Anisse Astier @ 2021-03-01 14:49 UTC (permalink / raw)
  To: Alexey Gladkov; +Cc: kbd

On Mon, Mar 01, 2021 at 03:09:39PM +0100, Alexey Gladkov wrote:
> On Sat, Feb 27, 2021 at 03:36:11PM +0100, Anisse Astier wrote:
> > The auto-generated (with ckbcomp) file fr-bepo_afnor did not load (even
> > partially), because of an U+1f12f (copyleft symbol) that is wrongly
> > parsed, generating this error message:
> > 
> > 	too many (160) entries on one line
> > 
> > Fix libkeymap so that the keymap can be parsed, even if the offending
> > character won't be loaded because of the ushort limitation of the
> > kb_value KDSKBENT uapi.
> > 
> > It's better to have the keymap partially loaded than not at all.
> 
> Nop. Partially keymap loading is very dangerous. You can get a completely
> unusable console. The libkeymap shouldn't break the console if it is known

By partially, I meant that only a single character of a single key
wouldn't load. I'm curious, what would be the implications here ? How
could it break ?

I tried loading this keymap, and didn't see any averse effect. The
character was missing, and that's about it. Why wouldn't we want that ?

> in advance that the keymap is not correct. You should fix ckbcomp so that
> it generates the correct keymap.
> 

I thought about doing this too, but it would mean not recognizing a
valid (albeit unsupported) unicode symbol. At least here the parsing
works, even if this particular symbol isn't loaded because of kernel
console limitations.

It feels weird to use the U+ unicode symbol notation, and then refuse to
load 2/3rd of those.

Regards,

Anisse



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

* Re: [kbd] [PATCH] src/libkeymap: add support for parsing more unicode values
  2021-03-01 14:49   ` Anisse Astier
@ 2021-03-02  0:47     ` Alexey Gladkov
  2021-03-03 17:09       ` [kbd] [PATCH] src/libkeymap: better error message on unsupported unicode value Anisse Astier
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Gladkov @ 2021-03-02  0:47 UTC (permalink / raw)
  To: Anisse Astier; +Cc: kbd

On Mon, Mar 01, 2021 at 03:49:52PM +0100, Anisse Astier wrote:
> On Mon, Mar 01, 2021 at 03:09:39PM +0100, Alexey Gladkov wrote:
> > On Sat, Feb 27, 2021 at 03:36:11PM +0100, Anisse Astier wrote:
> > > The auto-generated (with ckbcomp) file fr-bepo_afnor did not load (even
> > > partially), because of an U+1f12f (copyleft symbol) that is wrongly
> > > parsed, generating this error message:
> > > 
> > > 	too many (160) entries on one line
> > > 
> > > Fix libkeymap so that the keymap can be parsed, even if the offending
> > > character won't be loaded because of the ushort limitation of the
> > > kb_value KDSKBENT uapi.
> > > 
> > > It's better to have the keymap partially loaded than not at all.
> > 
> > Nop. Partially keymap loading is very dangerous. You can get a completely
> > unusable console. The libkeymap shouldn't break the console if it is known
> 
> By partially, I meant that only a single character of a single key
> wouldn't load. I'm curious, what would be the implications here ? How
> could it break ?

In this particular case, nothing will happen. An unicode character will
not be loaded and the user will probably see a warning. And this approach
is the best way to do it.

> I tried loading this keymap, and didn't see any averse effect. The
> character was missing, and that's about it. Why wouldn't we want that ?

>From the point of view of keymap processing in the kernel, such a keymap
is not valid. I believe that the library should report the fact of an
error in the keymap as early as possible.

Speaking of such restrictions, I would like to remind you that libkeymap
also has a restriction on the number of keys, but technically libkeymap
could parse more keys.

> > in advance that the keymap is not correct. You should fix ckbcomp so that
> > it generates the correct keymap.
> > 
> 
> I thought about doing this too, but it would mean not recognizing a
> valid (albeit unsupported) unicode symbol. At least here the parsing
> works, even if this particular symbol isn't loaded because of kernel
> console limitations.
> 
> It feels weird to use the U+ unicode symbol notation, and then refuse to
> load 2/3rd of those.

>From the point of view of the kernel, unicode symbols greater than 0xf000
are not valid.

-- 
Rgrds, legion



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

* [kbd] [PATCH] src/libkeymap: better error message on unsupported unicode value
  2021-03-02  0:47     ` Alexey Gladkov
@ 2021-03-03 17:09       ` Anisse Astier
  2021-03-03 18:59         ` Alexey Gladkov
  0 siblings, 1 reply; 6+ messages in thread
From: Anisse Astier @ 2021-03-03 17:09 UTC (permalink / raw)
  To: kbd; +Cc: Alexey Gladkov

The auto-generated (with ckbcomp) file fr-bepo_afnor did not load (even
partially), because of an U+1f12f (copyleft symbol) that is wrongly
parsed, generating this error message:
	too many (160) entries on one line

Fix libkeymap so that the symbol can be parsed, and later generate a
better error message:
	unicode keysym out of range: U+1f12f

At least users will know what is wrong with their keymap.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 src/libkeymap/analyze.l | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libkeymap/analyze.l b/src/libkeymap/analyze.l
index 9e76eae..4f9a6fa 100644
--- a/src/libkeymap/analyze.l
+++ b/src/libkeymap/analyze.l
@@ -319,7 +319,7 @@ Include			include[ \t]*
 Decimal			[1-9][0-9]*
 Octal			0[0-7]*
 Hex			0[xX][0-9a-fA-F]+
-Unicode			U\+([0-9a-fA-F]){4}
+Unicode			U\+([0-9a-fA-F]){4,6}
 Literal			[a-zA-Z][a-zA-Z_0-9]*
 Octa			([0-7]){1,3}
 Charset			charset|Charset|CharSet|CHARSET
-- 
2.29.2



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

* Re: [kbd] [PATCH] src/libkeymap: better error message on unsupported unicode value
  2021-03-03 17:09       ` [kbd] [PATCH] src/libkeymap: better error message on unsupported unicode value Anisse Astier
@ 2021-03-03 18:59         ` Alexey Gladkov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexey Gladkov @ 2021-03-03 18:59 UTC (permalink / raw)
  To: Anisse Astier; +Cc: kbd

On Wed, Mar 03, 2021 at 06:09:43PM +0100, Anisse Astier wrote:
> The auto-generated (with ckbcomp) file fr-bepo_afnor did not load (even
> partially), because of an U+1f12f (copyleft symbol) that is wrongly
> parsed, generating this error message:
> 	too many (160) entries on one line
> 
> Fix libkeymap so that the symbol can be parsed, and later generate a
> better error message:
> 	unicode keysym out of range: U+1f12f
> 
> At least users will know what is wrong with their keymap.
> 
> Signed-off-by: Anisse Astier <anisse@astier.eu>

Applied. Thanks!

> ---
>  src/libkeymap/analyze.l | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libkeymap/analyze.l b/src/libkeymap/analyze.l
> index 9e76eae..4f9a6fa 100644
> --- a/src/libkeymap/analyze.l
> +++ b/src/libkeymap/analyze.l
> @@ -319,7 +319,7 @@ Include			include[ \t]*
>  Decimal			[1-9][0-9]*
>  Octal			0[0-7]*
>  Hex			0[xX][0-9a-fA-F]+
> -Unicode			U\+([0-9a-fA-F]){4}
> +Unicode			U\+([0-9a-fA-F]){4,6}
>  Literal			[a-zA-Z][a-zA-Z_0-9]*
>  Octa			([0-7]){1,3}
>  Charset			charset|Charset|CharSet|CHARSET
> -- 
> 2.29.2
> 

-- 
Rgrds, legion



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

end of thread, other threads:[~2021-03-03 18:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-27 14:36 [kbd] [PATCH] src/libkeymap: add support for parsing more unicode values Anisse Astier
2021-03-01 14:09 ` Alexey Gladkov
2021-03-01 14:49   ` Anisse Astier
2021-03-02  0:47     ` Alexey Gladkov
2021-03-03 17:09       ` [kbd] [PATCH] src/libkeymap: better error message on unsupported unicode value Anisse Astier
2021-03-03 18:59         ` Alexey Gladkov

Linux console tools development discussion

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://lore.altlinux.org/kbd/0 kbd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kbd kbd/ http://lore.altlinux.org/kbd \
		kbd@lists.altlinux.org kbd@lists.altlinux.ru kbd@lists.altlinux.com
	public-inbox-index kbd

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://lore.altlinux.org/org.altlinux.lists.kbd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git