Re: [dev] [st] [PATCH] Fix techo handling of control and multibyte characters.

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.com>
Date: Tue, 29 Apr 2014 08:49:54 +0200

> > In file included from /usr/X11R6/include/X11/Xlib.h:47,
> > from st.c:25:
> > /usr/X11R6/include/X11/Xfuncproto.h:144:24: warning: ISO C does not permit named variadic macros
> > st.c: In function 'techo':
> > st.c:2303: warning: comparison is always false due to limited range of data type
> > st.c:2303: warning: comparison is always true due to limited range of data type
> > CC -o st
> > /usr/X11R6/lib/libX11.so.16.0: warning: strcpy() is almost always misused, please use strlcpy()
> > /usr/X11R6/lib/libX11.so.16.0: warning: strcat() is almost always misused, please use strlcat()
> > /usr/X11R6/lib/libX11.so.16.0: warning: sprintf() is often misused, please use snprintf()
> >
>

> That’s too vague. We don’t live in 1990 anymore, where people didn’t
> have POSIX around. The tips given by OpenBSD are useful and should be
> fixed. That’s why they are warnings.

Well, I strongly don't agree in some of this warnings, specially
in strcpy/strlcpy (please guys, I don't want to begin a flame war
about strcpy/strlcpy, it's only my personal opinion), and this is
the reason why I don't fix them. In the case of "comparision is
always...", is due to a macro that is build in order to work for
signed and unsigned numbers, so the only thing compiler must do
here is remove the unneded code and shut up. Another person who
agrees with me is Ken Thompson, who wrote in "A new compiler"
that conditional compilation should be replaced by "if" with
contstant values that compiler must remove in compile time.

> Alternative charsets? Why? I know you need much of that crap to have
> compatibility to old machines you are using. Why don’t you throw them
> away?

Well, the number of lines related to alternative charsets is less of
20 lines of code, so I don't think it can be called bloat. Alternative
charset are needed because there are terminfo entries that use them
(ac, smacs, rmacs) and they are used for graphical characteres, although
since st understand unicode, graphical characters could be sent directly
using their unicode values. After reviewing the code, we also can remove
some of the lines related to charset with this patch:

diff --git a/st.c b/st.c
index 5198749..0d1b674 100644
--- a/st.c
+++ b/st.c
_AT_@ -94,12 +94,11 @@ enum glyph_attribute {
         ATTR_REVERSE = 1,
         ATTR_UNDERLINE = 2,
         ATTR_BOLD = 4,
- ATTR_GFX = 8,
- ATTR_ITALIC = 16,
- ATTR_BLINK = 32,
- ATTR_WRAP = 64,
- ATTR_WIDE = 128,
- ATTR_WDUMMY = 256,
+ ATTR_ITALIC = 8,
+ ATTR_BLINK = 16,
+ ATTR_WRAP = 32,
+ ATTR_WIDE = 64,
+ ATTR_WDUMMY = 128,
 };
 
 enum cursor_movement {
_AT_@ -396,7 +395,6 @@ static void techo(char *, int);
 static bool tcontrolcode(uchar );
 static void tdectest(char );
 static int32_t tdefcolor(int *, int *, int);
-static void tselcs(void);
 static void tdeftran(char);
 static inline bool match(uint, uint);
 static void ttynew(void);
_AT_@ -1535,7 +1533,7 @@ tsetchar(char *c, Glyph *attr, int x, int y) {
         /*
          * The table is proudly stolen from rxvt.
          */
- if(attr->mode & ATTR_GFX) {
+ if(term.trantbl[term.charset] == CS_GRAPHIC0) {
                 if(BETWEEN(c[0], 0x41, 0x7e) && vt100_0[c[0] - 0x41]) {
                         c = vt100_0[c[0] - 0x41];
                 }
_AT_@ -2317,9 +2315,7 @@ 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', CS_GRAPHIC0}, {'B', CS_USA},
                 {0, 0}
         };
 
_AT_@ -2332,13 +2328,6 @@ tdeftran(char ascii) {
                 term.trantbl[term.icharset] = (*bp)[1];
 }
 
-void
-tselcs(void) {
- MODBIT(term.c.attr.mode,
- term.trantbl[term.charset] == CS_GRAPHIC0,
- ATTR_GFX);
-}
-
 bool
 tcontrolcode(uchar ascii) {
         static char question[UTF_SIZ] = "?";
_AT_@ -2377,11 +2366,9 @@ tcontrolcode(uchar ascii) {
                 return 1;
         case '\016': /* SO */
                 term.charset = 0;
- tselcs();
                 break;
         case '\017': /* SI */
                 term.charset = 1;
- tselcs();
                 break;
         case '\032': /* SUB */
                 tsetchar(question, &term.c.attr, term.c.x, term.c.y);
_AT_@ -2506,7 +2493,6 @@ tputc(char *c, int len) {
                         return;
                 } else if(term.esc & ESC_ALTCHARSET) {
                         tdeftran(ascii);
- tselcs();
                 } else if(term.esc & ESC_TEST) {
                         tdectest(ascii);
                 } else {
_AT_@ -2593,7 +2579,7 @@ tputc(char *c, int len) {
         /*
          * Display control codes only if we are in graphic mode
          */
- if(control && !(term.c.attr.mode & ATTR_GFX))
+ if(control && term.trantbl[term.charset] != CS_GRAPHIC0)
                 return;
         if(sel.ob.x != -1 && BETWEEN(term.c.y, sel.ob.y, sel.oe.y))
                 selclear(NULL);
-- 
Roberto E. Vargas Caballero
Received on Tue Apr 29 2014 - 08:49:54 CEST

This archive was generated by hypermail 2.3.0 : Tue Apr 29 2014 - 09:00:08 CEST