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

From: Christoph Lohmann <20h_AT_r-36.net>
Date: Tue, 29 Apr 2014 18:12:23 +0200

Greetings.

On Tue, 29 Apr 2014 18:12:23 +0200 "Roberto E. Vargas Caballero" <k0ga_AT_shike2.com> wrote:
> > > 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.

I will fix all warnings I encounter on my machines. I can use strlcpy()
and strcpy() correctly. sprintf() is of course unsafe.

> > 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:

That stuff does nothing. But as seen in many other parts of st, the
source is a bit of a documentary of the vtXXX days, with all kinds of
not implemented controls codes. I think it can stay. I asked the ques‐
tions about bloat to review feature additions a bit more, due to st be‐
ing nearly complete.


Sincerely,

Christoph Lohmann
Received on Tue Apr 29 2014 - 18:12:23 CEST