libvterm

Lots of compiler warnings, one significant.

Bug #1668780 reported by Edd Barrett
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
libvterm
Confirmed
Undecided
Unassigned

Bug Description

Hi,

We are in the process of importing libvterm into OpenBSD ports. Theo Buehler noticed the following potential bug thanks to a gcc warning:

```
    84 char c;
    85
    86 /* await CSI - 8bit or 2byte 7bit form */
    87 bool in_esc = false;
    88 while((c = getchar())) {
    89 if(c == 0x9b)
    90 break;
```

On some platforms chars are *signed*, and this may cause oddness. Here's the fix, use the correct return type of getchar():

```
--- bin/vterm-ctrl.c.orig Sun Dec 18 21:03:40 2016
+++ bin/vterm-ctrl.c Sat Feb 25 16:29:14 2017
@@ -81,7 +81,7 @@ static char *read_csi()
 {
   /* TODO: This really should be a more robust CSI parser
    */
- char c;
+ int c;

   /* await CSI - 8bit or 2byte 7bit form */
   bool in_esc = false;
```

Here are the remaining warnings on amd64:

```
TBL src/encoding/DECdrawing.tbl
TBL src/encoding/uk.tbl
CC src/encoding.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/encoding.c -fPIC -DPIC -o src/.libs/encoding.o
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/encoding.c -o src/encoding.o
CC src/keyboard.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/keyboard.c -fPIC -DPIC -o src/.libs/keyboard.o
src/keyboard.c: In function 'vterm_keyboard_key':
src/keyboard.c:133: warning: 'k.type' may be used uninitialized in this function
src/keyboard.c:133: warning: 'k.literal' may be used uninitialized in this function
src/keyboard.c:133: warning: 'k.csinum' may be used uninitialized in this function
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/keyboard.c -o src/keyboard.o
src/keyboard.c: In function 'vterm_keyboard_key':
src/keyboard.c:133: warning: 'k.type' may be used uninitialized in this function
src/keyboard.c:133: warning: 'k.literal' may be used uninitialized in this function
src/keyboard.c:133: warning: 'k.csinum' may be used uninitialized in this function
CC src/mouse.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/mouse.c -fPIC -DPIC -o src/.libs/mouse.o
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/mouse.c -o src/mouse.o
CC src/parser.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/parser.c -fPIC -DPIC -o src/.libs/parser.o
src/parser.c: In function 'vterm_input_write':
src/parser.c:193: warning: 'string_start' may be used uninitialized in this function
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/parser.c -o src/parser.o
src/parser.c: In function 'vterm_input_write':
src/parser.c:193: warning: 'string_start' may be used uninitialized in this function
CC src/pen.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/pen.c -fPIC -DPIC -o src/.libs/pen.o
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/pen.c -o src/pen.o
CC src/screen.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/screen.c -fPIC -DPIC -o src/.libs/screen.o
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/screen.c -o src/screen.o
CC src/state.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/state.c -fPIC -DPIC -o src/.libs/state.o
src/state.c: In function 'on_dcs':
src/state.c:1516: warning: 'reply' may be used uninitialized in this function
src/state.c:1516: note: 'reply' was declared here
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/state.c -o src/state.o
src/state.c: In function 'on_dcs':
src/state.c:1516: warning: 'reply' may be used uninitialized in this function
src/state.c:1516: note: 'reply' was declared here
CC src/unicode.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/unicode.c -fPIC -DPIC -o src/.libs/unicode.o
src/unicode.c:216: warning: 'mk_wcswidth' defined but not used
src/unicode.c:307: warning: 'mk_wcswidth_cjk' defined but not used
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/unicode.c -o src/unicode.o
src/unicode.c:216: warning: 'mk_wcswidth' defined but not used
src/unicode.c:307: warning: 'mk_wcswidth_cjk' defined but not used
CC src/vterm.c
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/vterm.c -fPIC -DPIC -o src/.libs/vterm.o
cc -O2 -pipe -Wall -Iinclude -std=c99 -c src/vterm.c -o src/vterm.o
LINK libvterm.la
libtool: link: cc -shared -fPIC -DPIC -o .libs/libvterm.so.0.0 src/.libs/encoding.o src/.libs/keyboard.o src/.libs/mouse.o src/.libs/parser.o src/.libs/pen.o src/.libs/screen.o src/.libs/state.o src/.libs/unicode.o src/.libs/vterm.o
libtool: link: ar cru .libs/libvterm.a src/encoding.o src/keyboard.o src/mouse.o src/parser.o src/pen.o src/screen.o src/state.o src/unicode.o src/vterm.o
libtool: link: ranlib .libs/libvterm.a
CC bin/unterm.c
libtool: link: cc -o bin/.libs/unterm -O2 -pipe -Wall -Iinclude -std=c99 bin/unterm.c -Lbin/.libs -lvterm -Wl,-rpath-link,/usr/local/lib
CC bin/vterm-ctrl.c
libtool: link: cc -o bin/.libs/vterm-ctrl -O2 -pipe -Wall -Iinclude -std=c99 bin/vterm-ctrl.c -Lbin/.libs -lvterm -Wl,-rpath-link,/usr/local/lib
CC bin/vterm-dump.c
libtool: link: cc -o bin/.libs/vterm-dump -O2 -pipe -Wall -Iinclude -std=c99 bin/vterm-dump.c -Lbin/.libs -lvterm -Wl,-rpath-link,/usr/local/lib
```

Thanks

Revision history for this message
Daniel Hahler (blueyed) wrote :

Currently on master/HEAD/tip (https://github.com/neovim/libvterm/archive/1aa95e24d8f07a396aa80b7cd52f93e2b5bcca79.tar.gz):

```
[6/9] Performing install step for 'libvterm'
make[1]: Entering directory '/home/daniel/Vcs/neovim/.deps/build/src/libvterm'
install -d /home/daniel/Vcs/neovim/.deps/usr/include
install -m644 include/vterm.h include/vterm_keycodes.h /home/daniel/Vcs/neovim/.deps/usr/include
install -d /home/daniel/Vcs/neovim/.deps/usr/lib/pkgconfig
sed -e "s,@PREFIX@,/home/daniel/Vcs/neovim/.deps/usr," -e "s,@LIBDIR@,/home/daniel/Vcs/neovim/.deps/usr/lib," -e "s,@VERSION@,0.1," <vterm.pc.in >/home/daniel/Vcs/neovim/.deps/usr/lib/pkgconfig/vterm.pc
TBL src/encoding/DECdrawing.tbl
TBL src/encoding/uk.tbl
CC src/encoding.c
CC src/keyboard.c
CC src/mouse.c
CC src/parser.c
CC src/pen.c
CC src/screen.c
CC src/state.c
CC src/unicode.c
src/unicode.c:215:12: warning: unused function 'mk_wcswidth' [-Wunused-function]
static int mk_wcswidth(const uint32_t *pwcs, size_t n)
           ^
src/unicode.c:306:12: warning: unused function 'mk_wcswidth_cjk' [-Wunused-function]
static int mk_wcswidth_cjk(const uint32_t *pwcs, size_t n)
           ^
2 warnings generated.
CC src/vterm.c
LINK libvterm.la
install -d /home/daniel/Vcs/neovim/.deps/usr/lib
libtool --quiet --mode=install install libvterm.la /home/daniel/Vcs/neovim/.deps/usr/lib/libvterm.la
libtool --quiet --mode=finish /home/daniel/Vcs/neovim/.deps/usr/lib
CC bin/unterm.c
bin/unterm.c:157:32: warning: suggest braces around initialization of subobject [-Wmissing-braces]
  VTermScreenCell prevcell = { 0 };
                               ^
                               {}
bin/unterm.c:175:32: warning: suggest braces around initialization of subobject [-Wmissing-braces]
  VTermScreenCell prevcell = { 0 };
                               ^
                               {}
2 warnings generated.
CC bin/vterm-ctrl.c
bin/vterm-ctrl.c:129:10: warning: result of comparison of constant 156 with expression of type 'char' is always false [-Wtautological-constant-out-of-range-compare]
    if(c == 0x9c) // ST
       ~ ^ ~~~~
1 warning generated.
CC bin/vterm-dump.c
install -d /home/daniel/Vcs/neovim/.deps/usr/bin
libtool --quiet --mode=install install bin/unterm bin/vterm-ctrl bin/vterm-dump /home/daniel/Vcs/neovim/.deps/usr/bin/
make[1]: Leaving directory '/home/daniel/Vcs/neovim/.deps/build/src/libvterm'

```

Changed in libvterm:
status: New → Confirmed
Revision history for this message
Travis Cole (calmkelp) wrote :

It looks like the code has drifted quite a lot since the above patch was submitted. This patch will apply cleanly on recent code:

diff --git bin/vterm-ctrl.c bin/vterm-ctrl.c
index ba0d61e..92a365f 100644
--- bin/vterm-ctrl.c
+++ bin/vterm-ctrl.c
@@ -79,9 +79,9 @@ static bool seticanon(bool icanon, bool echo)
   return ret;
 }

-static void await_c1(unsigned char c1)
+static void await_c1(int c1)
 {
- unsigned char c;
+ int c;

   /* await CSI - 8bit or 2byte 7bit form */
   bool in_esc = false;
@@ -106,7 +106,7 @@ static char *read_csi()
   char csi[32];
   int i = 0;
   for(; i < sizeof(csi)-1; i++) {
- char c = csi[i] = getchar();
+ int c = csi[i] = getchar();
     if(c >= 0x40 && c <= 0x7e)
       break;
   }
@@ -125,7 +125,7 @@ static char *read_dcs()
   bool in_esc = false;
   int i = 0;
   for(; i < sizeof(dcs)-1; ) {
- char c = getchar();
+ int c = getchar();
     if(c == 0x9c) // ST
       break;
     if(in_esc && c == 0x5c)

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.