On Wed, Jun 12, 2024 at 02:16:12PM +0200, Alejandro Colomar wrote:
> Hi!
>
> While compiling an example program in ioctl_tty(2) examples (now moving
> to TCSETS(2const)), I saw several warnings:
>
> tcgets.c:53:24: error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
> tio.c_cflag &= ~CBAUD;
> ~~ ^~~~~~
> tcgets.c:53:24: warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]
> tio.c_cflag &= ~CBAUD;
> ~~ ^~~~~~
> tcgets.c:58:24: error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka 'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
> tio.c_cflag &= ~(CBAUD << IBSHIFT);
> ~~ ^~~~~~~~~~~~~~~~~~~
> tcgets.c:58:24: warning: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise]
> tio.c_cflag &= ~(CBAUD << IBSHIFT);
> ~~ ^~~~~~~~~~~~~~~~~~~
> tcgets.c:58:25: warning: use of a signed integer operand with a unary bitwise operator [hicpp-signed-bitwise]
> tio.c_cflag &= ~(CBAUD << IBSHIFT);
> ~^~~~~~~~~~~~~~~~~~
>
> Which seem reasonable warnings. Bitwise operations on signed integers
> is bad for ye health. Let's see the definitions of struct termios:
>
> alx@debian:/usr/include$ grepc termios x86_64-linux-gnu/ asm-generic/
> x86_64-linux-gnu/bits/termios-struct.h:struct termios
> {
> tcflag_t c_iflag; /* input mode flags */
> tcflag_t c_oflag; /* output mode flags */
> tcflag_t c_cflag; /* control mode flags */
> tcflag_t c_lflag; /* local mode flags */
> cc_t c_line; /* line discipline */
> cc_t c_cc[NCCS]; /* control characters */
> speed_t c_ispeed; /* input speed */
> speed_t c_ospeed; /* output speed */
> #define _HAVE_STRUCT_TERMIOS_C_ISPEED 1
> #define _HAVE_STRUCT_TERMIOS_C_OSPEED 1
> };
> asm-generic/termbits.h:struct termios {
> tcflag_t c_iflag; /* input mode flags */
> tcflag_t c_oflag; /* output mode flags */
> tcflag_t c_cflag; /* control mode flags */
> tcflag_t c_lflag; /* local mode flags */
> cc_t c_line; /* line discipline */
> cc_t c_cc[NCCS]; /* control characters */
> };
>
> Except for the speed members, they seem the same. The types of the
> members are correctly unsigned:
>
> alx@debian:/usr/include$ grepc tcflag_t x86_64-linux-gnu/ asm-generic/
> x86_64-linux-gnu/bits/termios.h:typedef unsigned int tcflag_t;
> asm-generic/termbits.h:typedef unsigned int tcflag_t;
>
> So, all of the constants that are to be used in these members, which
> will undergo bitwise operations, should use the U suffix. Here's the
> list for the UAPI:
>
> $ sed -n '/flag bits/,/^$/p' asm-generic/termbits.h
> /* c_iflag bits */
> #define IUCLC 0x0200
> #define IXON 0x0400
> #define IXOFF 0x1000
> #define IMAXBEL 0x2000
> #define IUTF8 0x4000
>
> /* c_oflag bits */
> #define OLCUC 0x00002
> #define ONLCR 0x00004
> #define NLDLY 0x00100
> #define NL0 0x00000
> #define NL1 0x00100
> #define CRDLY 0x00600
> #define CR0 0x00000
> #define CR1 0x00200
> #define CR2 0x00400
> #define CR3 0x00600
> #define TABDLY 0x01800
> #define TAB0 0x00000
> #define TAB1 0x00800
> #define TAB2 0x01000
> #define TAB3 0x01800
> #define XTABS 0x01800
> #define BSDLY 0x02000
> #define BS0 0x00000
> #define BS1 0x02000
> #define VTDLY 0x04000
> #define VT0 0x00000
> #define VT1 0x04000
> #define FFDLY 0x08000
> #define FF0 0x00000
> #define FF1 0x08000
>
> /* c_lflag bits */
> #define ISIG 0x00001
> #define ICANON 0x00002
> #define XCASE 0x00004
> #define ECHO 0x00008
> #define ECHOE 0x00010
> #define ECHOK 0x00020
> #define ECHONL 0x00040
> #define NOFLSH 0x00080
> #define TOSTOP 0x00100
> #define ECHOCTL 0x00200
> #define ECHOPRT 0x00400
> #define ECHOKE 0x00800
> #define FLUSHO 0x01000
> #define PENDIN 0x04000
> #define IEXTEN 0x08000
> #define EXTPROC 0x10000
>
> And glibc also (re)defines some of them, so those should also have the
> U suffix.
>
> Any opinions?
Have a proposed patch that you feel would resolve this?
thanks,
greg k-h
