Re: [dev] [st] [PATCH] More compatible with xterm
> > It look like problem not related to save/restore cursor position. This > test have strange sequence: ESC[m ESC(B ESC)B > What this sequence should do? ESC[m = SGR. Parameter=0 restore default graphic rendering values ESC(B = SCS. Selects USASCII in primary charset (G0) ESC)B = SCS. Selects USASCII in secondary charset (G1) I can see now that the problem is related in how we ignore ESC)B. We don't implement national charsets because we support only utf8. The only exception is the graphic charset that is heavily used. It means we implement only the G0 charset. When we receive a ESC( we read the next character and only we do something useful in ESC(0 or ESC(B. But when we receive ESC), since we don't implement G1 at all, we stop reading the escape sequence, and we don't ignore the next character in the stream, the B. I will submit a fast patch for this issue. Beswt regards, -- Roberto E. Vargas Caballero k...@shike2.com http://www.shike2.com
[dev] [st][PATCH] Add support for multiple charset definitions
vt100 has support for two defined charset, G0 and G1. Each charset
can be defined, but in each moment is selected only one of both
charset. This is usually used selecting a national charset in G0
and graphic charset in G1, so you can switch between graphic
charset and text charset without losing the national charset
already defined.
st hasn't support for national charsets, because it is an utf8
based terminal emulator, but it has support for graphic
charset because it is heavily used, but it only supports G0,
without understanding G1 selection sequences, which causes some
programs in some moments can print some garbage in the screen.
This patch adds a fake support for multiple charset definitions,
where we only support graphic charset and us-ascii charset, but
we allow more of one charset definition.
This patch allow define G0 until G3 charsets, but only accepts
select G0 or G1, and it accepts some national charset definitions
but all of them are mapped to us-ascii.
---
st.c | 78 ++--
1 file changed, 53 insertions(+), 25 deletions(-)
diff --git a/st.c b/st.c
index 12fcc90..c752906 100644
--- a/st.c
+++ b/st.c
@@ -137,6 +137,16 @@ enum term_mode {
|MODE_MOUSEMANY,
};
+enum charset {
+ CS_GRAPHIC0,
+ CS_GRAPHIC1,
+ CS_UK,
+ CS_USA,
+ CS_MULTI,
+ CS_GER,
+ CS_FIN
+};
+
enum escape_state {
ESC_START = 1,
ESC_CSI= 2,
@@ -216,6 +226,9 @@ typedef struct {
int bot; /* bottom scroll limit */
int mode; /* terminal mode flags */
int esc; /* escape state flags */
+ char trantbl[4]; /* charset table translation */
+ int charset; /* current charset */
+ int icharset; /* selected charset for sequence */
bool numlock; /* lock numbers in keyboard */
bool *tabs;
} Term;
@@ -367,6 +380,8 @@ static void tsetmode(bool, bool, int *, int);
static void tfulldirt(void);
static void techo(char *, int);
static long tdefcolor(int *, int *, int);
+static void tsetchar(void);
+static void tdeftran(char);
static inline bool match(uint, uint);
static void ttynew(void);
static void ttyread(void);
@@ -1369,6 +1384,8 @@ treset(void) {
term.top = 0;
term.bot = term.row - 1;
term.mode = MODE_WRAP;
+ memset(term.trantbl, sizeof(term.trantbl), CS_USA);
+ term.charset = 0;
tclearregion(0, 0, term.col-1, term.row-1);
tmoveto(0, 0);
@@ -2257,6 +2274,33 @@ techo(char *buf, int len) {
}
void
+tdeftran(char ascii) {
+ char c, (*bp)[2];
+ static char tbl[][2] = {
+ {'0', CS_GRAPHIC0}, {'1', CS_GRAPHIC1}, {'A', CS_UK},
+ {'B', CS_USA}, {'<', CS_MULTI},{'K', CS_GER},
+ {'5', CS_FIN}, {'C', CS_FIN},
+ {0, 0}
+ };
+
+ for (bp = &tbl[0]; (c = (*bp)[0]) && c != ascii; ++bp)
+ /* nothing */;
+
+ if (c == 0)
+ fprintf(stderr, "esc unhandled charset: ESC ( %c\n", ascii);
+ else
+ term.trantbl[term.icharset] = (*bp)[1];
+}
+
+void
+tsetchar(void) {
+ if (term.trantbl[term.charset] == CS_GRAPHIC0)
+ term.c.attr.mode |= ATTR_GFX;
+ else
+ term.c.attr.mode &= ~ATTR_GFX;
+}
+
+void
tputc(char *c, int len) {
uchar ascii = *c;
bool control = ascii < '\x20' || ascii == 0177;
@@ -2348,13 +2392,12 @@ tputc(char *c, int len) {
term.esc = ESC_START;
return;
case '\016': /* SO */
+ term.charset = 0;
+ tsetchar();
+ return;
case '\017': /* SI */
- /*
-* Different charsets are hard to handle. Applications
-* should use the right alt charset escapes for the
-* only reason they still exist: line drawing. The
-* rest is incompatible history st should not support.
-*/
+ term.charset = 1;
+ tsetchar();
return;
case '\032': /* SUB */
case '\030': /* CAN */
@@ -2382,22 +2425,8 @@ tputc(char *c, int len) {
if(ascii == '\\')
strhandle();
} else if(term.esc & ESC_ALTCHARSET) {
- switch(ascii) {
- case '0': /* Line drawing set */
- term.c.attr.mode |= ATTR_GFX;
- break;
- case 'B': /* USASCII */
- term.c.attr.mode &= ~ATTR_GFX;
- break;
- case 'A': /* UK (IGNORED) */
- case '<': /* multinational charset (
[dev] Re: [st] [PATCH] Avoid buffer overflows in the case of key-mapped strings.
On Mon, 23 Sep 2013 10:06:13 +0200, Roberto E. Vargas Caballero wrote:
> On Sat, Sep 21, 2013 at 03:41:01PM +0200, Mark Edgar wrote:
> I don't agree here, because if you insert a escape sequence is not dificult
> get more of 16 characters (for example a key combination which updates the
> bar title).
Yes, agreed.
> > +#define STRNLEN(s) ((s)[LEN((s)) - 1] == '\0' ? strlen((s)) : LEN((s)))
> This trick is only useful when you usually have strings with a size of
> LEN(s)-1, but in our case we will usually have string with a small size,
> so it means we always will add a new test before of calling to strlen.
This test here was to ensure the string is NUL-terminated. With a
16-char length buffer, it's more likely that someone will accidentally
use it all. In other words, this Key entry is legal, but strlen()
can't handle it:
static Key key[] = {
{ XK_F12, XK_NO_MOD, "0123456789012345" },
With the current version of st, that string literal would have to be
512 characters long before overflow. This is unlikely, but still
possible.
> Could you explain what overflow are you fixing here?, because I only
> can see you are returning the Key struct instead of returning directly
> the char representation.
There's actually two here: the strlen(customkey) I mentioned above,
and the original memcpy(buf[32], customkey[512], len<512) call in
kpress(). The following key entry will overflow buf[] in kpress() in
the current version of st. If it doesn't go boom for you, try doubling
the length of the string literal:
static Key key[] = {
{ XK_F12, XK_NO_MOD, "01234567890123456789012345678901" },
A much neater fix might be to make Key.s a char pointer instead. This
also solves the question of what to use as the string length limit
(should it be 16 or 32 or 512 -- well, how about unlimited?), avoids
invalid strlen() calls (assuming config.h doesn't do anything too
tricky), and entirely avoids the extra memcpy():
--- a/st.c
+++ b/st.c
@@ -250,7 +250,7 @@ typedef struct {
typedef struct {
KeySym k;
uint mask;
- char s[ESC_BUF_SIZ];
+ char *s;
/* three valued logic variables: 0 indifferent, 1 on, -1 off */
signed char keypad; /* application keypad */
signed char cursor; /* application cursor */
@@ -3538,7 +3538,10 @@ kpress(XEvent *ev) {
/* 2. custom keys from config.h */
if((customkey = kmap(ksym, e->state))) {
len = strlen(customkey);
- memcpy(buf, customkey, len);
+ ttywrite(customkey, len);
+ if(IS_SET(MODE_ECHO))
+ techo(customkey, len);
+ return;
/* 3. hardcoded (overrides X lookup) */
} else {
if(len == 0)
What do you think?
FYI: I have another patch which allows for keys to send the NUL byte
to the tty, but I'm not really sure how generally useful it really is.
It would look like this in config.h:
static Key key[] = {
{ XK_F12, XK_NO_MOD, "\0", .len=1 },
-Mark
Re: [dev] Re: [st] [PATCH] Avoid buffer overflows in the case of key-mapped strings.
2013/10/2 Mark Edgar :
> On Mon, 23 Sep 2013 10:06:13 +0200, Roberto E. Vargas Caballero wrote:
>> On Sat, Sep 21, 2013 at 03:41:01PM +0200, Mark Edgar wrote:
>> I don't agree here, because if you insert a escape sequence is not dificult
>> get more of 16 characters (for example a key combination which updates the
>> bar title).
>
> Yes, agreed.
>
>
>> > +#define STRNLEN(s) ((s)[LEN((s)) - 1] == '\0' ? strlen((s)) : LEN((s)))
>> This trick is only useful when you usually have strings with a size of
>> LEN(s)-1, but in our case we will usually have string with a small size,
>> so it means we always will add a new test before of calling to strlen.
>
> This test here was to ensure the string is NUL-terminated. With a
> 16-char length buffer, it's more likely that someone will accidentally
> use it all. In other words, this Key entry is legal, but strlen()
> can't handle it:
>
> static Key key[] = {
> { XK_F12, XK_NO_MOD, "0123456789012345" },
>
> With the current version of st, that string literal would have to be
> 512 characters long before overflow. This is unlikely, but still
> possible.
>
>
>> Could you explain what overflow are you fixing here?, because I only
>> can see you are returning the Key struct instead of returning directly
>> the char representation.
>
> There's actually two here: the strlen(customkey) I mentioned above,
> and the original memcpy(buf[32], customkey[512], len<512) call in
> kpress(). The following key entry will overflow buf[] in kpress() in
> the current version of st. If it doesn't go boom for you, try doubling
> the length of the string literal:
>
> static Key key[] = {
> { XK_F12, XK_NO_MOD, "01234567890123456789012345678901" },
>
>
> A much neater fix might be to make Key.s a char pointer instead. This
> also solves the question of what to use as the string length limit
> (should it be 16 or 32 or 512 -- well, how about unlimited?), avoids
> invalid strlen() calls (assuming config.h doesn't do anything too
> tricky), and entirely avoids the extra memcpy():
>
> --- a/st.c
> +++ b/st.c
> @@ -250,7 +250,7 @@ typedef struct {
> typedef struct {
> KeySym k;
> uint mask;
> - char s[ESC_BUF_SIZ];
> + char *s;
> /* three valued logic variables: 0 indifferent, 1 on, -1 off */
> signed char keypad; /* application keypad */
> signed char cursor; /* application cursor */
> @@ -3538,7 +3538,10 @@ kpress(XEvent *ev) {
> /* 2. custom keys from config.h */
> if((customkey = kmap(ksym, e->state))) {
> len = strlen(customkey);
> - memcpy(buf, customkey, len);
> + ttywrite(customkey, len);
> + if(IS_SET(MODE_ECHO))
> + techo(customkey, len);
> + return;
> /* 3. hardcoded (overrides X lookup) */
> } else {
> if(len == 0)
>
>
> What do you think?
>
>
> FYI: I have another patch which allows for keys to send the NUL byte
> to the tty, but I'm not really sure how generally useful it really is.
> It would look like this in config.h:
>
> static Key key[] = {
> { XK_F12, XK_NO_MOD, "\0", .len=1 },
>
> -Mark
>
Isn't there strnlen() function?
Re: [dev] [st][PATCH] Add support for multiple charset definitions
Hi Roberto
On Tue, Oct 01, 2013 at 09:32:16PM +0200, Roberto E. Vargas Caballero wrote:
> vt100 has support for two defined charset, G0 and G1. Each charset
> can be defined, but in each moment is selected only one of both
> charset. This is usually used selecting a national charset in G0
> and graphic charset in G1, so you can switch between graphic
> charset and text charset without losing the national charset
> already defined.
>
> st hasn't support for national charsets, because it is an utf8
> based terminal emulator, but it has support for graphic
> charset because it is heavily used, but it only supports G0,
> without understanding G1 selection sequences, which causes some
> programs in some moments can print some garbage in the screen.
>
> This patch adds a fake support for multiple charset definitions,
> where we only support graphic charset and us-ascii charset, but
> we allow more of one charset definition.
>
> This patch allow define G0 until G3 charsets, but only accepts
> select G0 or G1, and it accepts some national charset definitions
> but all of them are mapped to us-ascii.
>
> [...]
I applied the patch to tip ( eeae9b0 ) but compilation fails due to
a redefinition of a the tsetchar function:
st.c:383:13: error: conflicting types for ‘tsetchar’
static void tsetchar(void);
^
st.c:374:13: note: previous declaration of ‘tsetchar’ was here
static void tsetchar(char *, Glyph *, int, int);
^
st.c:1548:1: error: conflicting types for ‘tsetchar’
tsetchar(char *c, Glyph *attr, int x, int y) {
^
st.c:383:13: note: previous declaration of ‘tsetchar’ was here
static void tsetchar(void);
^
st.c:2288:1: error: conflicting types for ‘tsetchar’
tsetchar(void) {
^
st.c:1548:1: note: previous definition of ‘tsetchar’ was here
tsetchar(char *c, Glyph *attr, int x, int y) {
^
st.c: In function ‘tputc’:
st.c:2430:7: error: too many arguments to function ‘tsetchar’
tsetchar(E, &term.c.attr, x, y);
^
st.c:2288:1: note: declared here
tsetchar(void) {
^
st.c:2544:2: error: too many arguments to function ‘tsetchar’
tsetchar(c, &term.c.attr, term.c.x, term.c.y);
^
st.c:2288:1: note: declared here
tsetchar(void) {
^
make: *** [st.o] Error 1
Am I missing something?
Cheers,
Silvan
Re: [dev] [st][PATCH] Add support for multiple charset definitions
> I applied the patch to tip ( eeae9b0 ) but compilation fails due to
> a redefinition of a the tsetchar function:
Sorry, I don't know why my gcc version doesn't give me the error (some
type of overloading). This version uses a different name for this
new function.
-- >8 --
Subject: [PATCH] Add support for multiple charset definitions
vt100 has support for two defined charset, G0 and G1. Each charset
can be defined, but in each moment is selected only one of both
charset. This is usually used selecting a national charset in G0
and graphic charset in G1, so you can switch between graphic
charset and text charset without losing the national charset
already defined.
st hasn't support for national charsets, because it is an utf8
based terminal emulator, but it has support for graphic
charset because it is heavily used, but it only supports G0,
without understanding G1 selection sequences, which causes some
programs in some moments can print some garbage in the screen.
This patch adds a fake support for multiple charset definitions,
where we only support graphic charset and us-ascii charset, but
we allow more of one charset definition.
This patch allow define G0 until G3 charsets, but only accepts
select G0 or G1, and it accepts some national charset definitions
but all of them are mapped to us-ascii.
---
st.c | 78 ++--
1 file changed, 53 insertions(+), 25 deletions(-)
diff --git a/st.c b/st.c
index 12fcc90..a22ba97 100644
--- a/st.c
+++ b/st.c
@@ -137,6 +137,16 @@ enum term_mode {
|MODE_MOUSEMANY,
};
+enum charset {
+ CS_GRAPHIC0,
+ CS_GRAPHIC1,
+ CS_UK,
+ CS_USA,
+ CS_MULTI,
+ CS_GER,
+ CS_FIN
+};
+
enum escape_state {
ESC_START = 1,
ESC_CSI= 2,
@@ -216,6 +226,9 @@ typedef struct {
int bot; /* bottom scroll limit */
int mode; /* terminal mode flags */
int esc; /* escape state flags */
+ char trantbl[4]; /* charset table translation */
+ int charset; /* current charset */
+ int icharset; /* selected charset for sequence */
bool numlock; /* lock numbers in keyboard */
bool *tabs;
} Term;
@@ -367,6 +380,8 @@ static void tsetmode(bool, bool, int *, int);
static void tfulldirt(void);
static void techo(char *, int);
static long tdefcolor(int *, int *, int);
+static void tselcs(void);
+static void tdeftran(char);
static inline bool match(uint, uint);
static void ttynew(void);
static void ttyread(void);
@@ -1369,6 +1384,8 @@ treset(void) {
term.top = 0;
term.bot = term.row - 1;
term.mode = MODE_WRAP;
+ memset(term.trantbl, sizeof(term.trantbl), CS_USA);
+ term.charset = 0;
tclearregion(0, 0, term.col-1, term.row-1);
tmoveto(0, 0);
@@ -2257,6 +2274,33 @@ techo(char *buf, int len) {
}
void
+tdeftran(char ascii) {
+ char c, (*bp)[2];
+ static char tbl[][2] = {
+ {'0', CS_GRAPHIC0}, {'1', CS_GRAPHIC1}, {'A', CS_UK},
+ {'B', CS_USA}, {'<', CS_MULTI},{'K', CS_GER},
+ {'5', CS_FIN}, {'C', CS_FIN},
+ {0, 0}
+ };
+
+ for (bp = &tbl[0]; (c = (*bp)[0]) && c != ascii; ++bp)
+ /* nothing */;
+
+ if (c == 0)
+ fprintf(stderr, "esc unhandled charset: ESC ( %c\n", ascii);
+ else
+ term.trantbl[term.icharset] = (*bp)[1];
+}
+
+void
+tselcs(void) {
+ if (term.trantbl[term.charset] == CS_GRAPHIC0)
+ term.c.attr.mode |= ATTR_GFX;
+ else
+ term.c.attr.mode &= ~ATTR_GFX;
+}
+
+void
tputc(char *c, int len) {
uchar ascii = *c;
bool control = ascii < '\x20' || ascii == 0177;
@@ -2348,13 +2392,12 @@ tputc(char *c, int len) {
term.esc = ESC_START;
return;
case '\016': /* SO */
+ term.charset = 0;
+ tselcs();
+ return;
case '\017': /* SI */
- /*
-* Different charsets are hard to handle. Applications
-* should use the right alt charset escapes for the
-* only reason they still exist: line drawing. The
-* rest is incompatible history st should not support.
-*/
+ term.charset = 1;
+ tselcs();
return;
case '\032': /* SUB */
case '\030': /* CAN */
@@ -2382,22 +2425,8 @@ tputc(char *c, int len) {
if(ascii == '\\')
strhandle();
} else if(term.esc & ESC_ALTCHARSET) {
- switch(ascii) {
- case '0': /* Line drawing set */
-
Re: [dev] [st][PATCH] Add support for multiple charset definitions
On Tue, Oct 01, 2013 at 10:33:40PM +0200, Roberto E. Vargas Caballero wrote: > > I applied the patch to tip ( eeae9b0 ) but compilation fails due to > > a redefinition of a the tsetchar function: > > Sorry, I don't know why my gcc version doesn't give me the error (some > type of overloading). This version uses a different name for this > new function. As function overloading does not seem to be supported by C I have no idea why you did not get any errors... > -- >8 -- > > Subject: [PATCH] Add support for multiple charset definitions > > [...] This one compiled without a problem. Thanks! Cheers, Silvan
Re: [dev] Re: [st] [PATCH] Avoid buffer overflows in the case of key-mapped strings.
> >> > +#define STRNLEN(s) ((s)[LEN((s)) - 1] == '\0' ? strlen((s)) : LEN((s)))
> >> This trick is only useful when you usually have strings with a size of
> >> LEN(s)-1, but in our case we will usually have string with a small size,
> >> so it means we always will add a new test before of calling to strlen.
> >
> > This test here was to ensure the string is NUL-terminated. With a
> > 16-char length buffer, it's more likely that someone will accidentally
> > use it all. In other words, this Key entry is legal, but strlen()
> > can't handle it:
Ok, I can see now that Key.s is an array and not a pointer. We have a
problem with this definition.
> > A much neater fix might be to make Key.s a char pointer instead. This
> > also solves the question of what to use as the string length limit
> > (should it be 16 or 32 or 512 -- well, how about unlimited?), avoids
> > invalid strlen() calls (assuming config.h doesn't do anything too
> > tricky), and entirely avoids the extra memcpy():
> > @@ -3538,7 +3538,10 @@ kpress(XEvent *ev) {
> > /* 2. custom keys from config.h */
> > if((customkey = kmap(ksym, e->state))) {
> > len = strlen(customkey);
> > - memcpy(buf, customkey, len);
> > + ttywrite(customkey, len);
> > + if(IS_SET(MODE_ECHO))
> > + techo(customkey, len);
> > + return;
> > /* 3. hardcoded (overrides X lookup) */
> > } else {
> > if(len == 0)
I agree here, good catch and good implementation. Could you send
a mail with the patch and a proper commit message?
> > FYI: I have another patch which allows for keys to send the NUL byte
> > to the tty, but I'm not really sure how generally useful it really is.
> > It would look like this in config.h:
> >
> > static Key key[] = {
> > { XK_F12, XK_NO_MOD, "\0", .len=1 },
> >
I cannot see when could be interesting send a '\0'. It someone can give
a case when it is necessary we can think about this patch.
> Isn't there strnlen() function?
This was the same I was thinking before of reading this new version,
but it is not necessary now.
--
Roberto E. Vargas Caballero
k...@shike2.com
http://www.shike2.com
Re: [dev] [st] [PATCH] Avoid buffer overflows in the case of key-mapped strings.
>> Isn't there strnlen() function? > > This was the same I was thinking before of reading this new version, > but it is not necessary now. I was thinking the same until I checked back on the ISO C11 specification and - there's no strnlen function :D ~koneu
Re: [dev] [st] [PATCH] Avoid buffer overflows in the case of key-mapped strings.
> I was thinking the same until I checked back on the ISO C11 specification and > - there's no strnlen function :D Yes, it is true, but take a look to strnlen(3): The strlen() and strnlen() functions conform to IEEE Std 1003.1-2008 (``POSIX.1''). ;). -- Roberto E. Vargas Caballero k...@shike2.com http://www.shike2.com
Re: [dev] Fwd: mod patrick295767 - adding ~/.dwmrc
On Sat, Sep 28, 2013 at 11:45 PM, patrick295767 patrick295767 wrote: > Hi, > > I would like to share a mod of the config.h and dwm.c. > > Please create a file ~/.dwmrc with its content, before running dwm: >xterm -bg blue -fg yellow > > This mod uses the window key flag, which is used because I like to keep alt > for other applications working. > > In this config, you may try: >mod4 + l >mod4 + h > I am sure that it may be useful. > > mod4 + w to close/kill > mod4 + x to read the cfg and launch xterm > > Nice dwm, All the best, > Uncle Pat > -- > vim + dwm + gcc are the best > and the console ;) > IMNSHO, this should go to the patch collection on sites. The wiki is public after all. cheers! mar77i
