termios constants should be unsigned

termios constants should be unsigned

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 4114 bytes --]

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 lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: termios constants should be unsigned

[Greg KH]

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

[PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 34853 bytes --]

Constants that are to be used in bitwise operations should be unsigned,
or a user could easily trigger Undefined Behavior.
Also, the types where these constants are to be assigned are unsigned,
so this makes it more consistent.

alx@debian:/usr/include$ grepc -tt termios asm-generic/
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 */
};
alx@debian:/usr/include$ grepc -tt tcflag_t asm-generic/
asm-generic/termbits.h:typedef unsigned int	tcflag_t;
alx@debian:/usr/include$ grepc -tt cc_t asm-generic/
asm-generic/termbits-common.h:typedef unsigned char	cc_t;
alx@debian:/usr/include$ grepc -tt speed_t asm-generic/
asm-generic/termbits-common.h:typedef unsigned int	speed_t;

Link: <https://lore.kernel.org/linux-api/2024061222-scuttle-expanse-6438@gregkh/T/>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Palmer Dabbelt <palmer@rivosinc.com>
Cc: <linux-api@vger.kernel.org>
Cc: <libc-alpha@sourceware.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

Hi Greg,

On Wed, Jun 12, 2024 at 02:22:37PM GMT, Greg KH wrote:
> Have a proposed patch that you feel would resolve this?
>
> thanks,
>
> greg k-h

Here it is.  :)

For reviewing it, I suggest using '--word-diff-regex=.'.

I compiled the kernel, and it seems ok; didn't test more than that.

Have a lovely day!
Alex

Range-diff:
-:  ------------ > 1:  588eb01ac153 uapi/asm/termbits: Use the U integer suffix for bit fields

 arch/alpha/include/uapi/asm/termbits.h   | 154 +++++++++---------
 arch/mips/include/uapi/asm/termbits.h    | 154 +++++++++---------
 arch/parisc/include/uapi/asm/termbits.h  | 152 +++++++++---------
 arch/powerpc/include/uapi/asm/termbits.h | 156 +++++++++---------
 arch/sparc/include/uapi/asm/termbits.h   | 192 +++++++++++------------
 include/uapi/asm-generic/termbits.h      | 152 +++++++++---------
 6 files changed, 480 insertions(+), 480 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/termbits.h b/arch/alpha/include/uapi/asm/termbits.h
index f1290b22072b..965a8c93523d 100644
--- a/arch/alpha/include/uapi/asm/termbits.h
+++ b/arch/alpha/include/uapi/asm/termbits.h
@@ -70,39 +70,39 @@ struct ktermios {
 #define VTIME		17
 
 /* c_iflag bits */
-#define IXON	0x0200
-#define IXOFF	0x0400
-#define IUCLC	0x1000
-#define IMAXBEL	0x2000
-#define IUTF8	0x4000
+#define IXON	0x0200U
+#define IXOFF	0x0400U
+#define IUCLC	0x1000U
+#define IMAXBEL	0x2000U
+#define IUTF8	0x4000U
 
 /* c_oflag bits */
-#define ONLCR	0x00002
-#define OLCUC	0x00004
-#define NLDLY	0x00300
-#define   NL0	0x00000
-#define   NL1	0x00100
-#define   NL2	0x00200
-#define   NL3	0x00300
-#define TABDLY	0x00c00
-#define   TAB0	0x00000
-#define   TAB1	0x00400
-#define   TAB2	0x00800
-#define   TAB3	0x00c00
-#define CRDLY	0x03000
-#define   CR0	0x00000
-#define   CR1	0x01000
-#define   CR2	0x02000
-#define   CR3	0x03000
-#define FFDLY	0x04000
-#define   FF0	0x00000
-#define   FF1	0x04000
-#define BSDLY	0x08000
-#define   BS0	0x00000
-#define   BS1	0x08000
-#define VTDLY	0x10000
-#define   VT0	0x00000
-#define   VT1	0x10000
+#define ONLCR	0x00002U
+#define OLCUC	0x00004U
+#define NLDLY	0x00300U
+#define   NL0	0x00000U
+#define   NL1	0x00100U
+#define   NL2	0x00200U
+#define   NL3	0x00300U
+#define TABDLY	0x00c00U
+#define   TAB0	0x00000U
+#define   TAB1	0x00400U
+#define   TAB2	0x00800U
+#define   TAB3	0x00c00U
+#define CRDLY	0x03000U
+#define   CR0	0x00000U
+#define   CR1	0x01000U
+#define   CR2	0x02000U
+#define   CR3	0x03000U
+#define FFDLY	0x04000U
+#define   FF0	0x00000U
+#define   FF1	0x04000U
+#define BSDLY	0x08000U
+#define   BS0	0x00000U
+#define   BS1	0x08000U
+#define VTDLY	0x10000U
+#define   VT0	0x00000U
+#define   VT1	0x10000U
 /*
  * Should be equivalent to TAB3, see description of TAB3 in
  * POSIX.1-2008, Ch. 11.2.3 "Output Modes"
@@ -110,54 +110,54 @@ struct ktermios {
 #define XTABS	TAB3
 
 /* c_cflag bit meaning */
-#define CBAUD		0x0000001f
-#define CBAUDEX		0x00000000
-#define BOTHER		0x0000001f
-#define     B57600	0x00000010
-#define    B115200	0x00000011
-#define    B230400	0x00000012
-#define    B460800	0x00000013
-#define    B500000	0x00000014
-#define    B576000	0x00000015
-#define    B921600	0x00000016
-#define   B1000000	0x00000017
-#define   B1152000	0x00000018
-#define   B1500000	0x00000019
-#define   B2000000	0x0000001a
-#define   B2500000	0x0000001b
-#define   B3000000	0x0000001c
-#define   B3500000	0x0000001d
-#define   B4000000	0x0000001e
-#define CSIZE		0x00000300
-#define   CS5		0x00000000
-#define   CS6		0x00000100
-#define   CS7		0x00000200
-#define   CS8		0x00000300
-#define CSTOPB		0x00000400
-#define CREAD		0x00000800
-#define PARENB		0x00001000
-#define PARODD		0x00002000
-#define HUPCL		0x00004000
-#define CLOCAL		0x00008000
-#define CIBAUD		0x001f0000
+#define CBAUD		0x0000001fU
+#define CBAUDEX		0x00000000U
+#define BOTHER		0x0000001fU
+#define     B57600	0x00000010U
+#define    B115200	0x00000011U
+#define    B230400	0x00000012U
+#define    B460800	0x00000013U
+#define    B500000	0x00000014U
+#define    B576000	0x00000015U
+#define    B921600	0x00000016U
+#define   B1000000	0x00000017U
+#define   B1152000	0x00000018U
+#define   B1500000	0x00000019U
+#define   B2000000	0x0000001aU
+#define   B2500000	0x0000001bU
+#define   B3000000	0x0000001cU
+#define   B3500000	0x0000001dU
+#define   B4000000	0x0000001eU
+#define CSIZE		0x00000300U
+#define   CS5		0x00000000U
+#define   CS6		0x00000100U
+#define   CS7		0x00000200U
+#define   CS8		0x00000300U
+#define CSTOPB		0x00000400U
+#define CREAD		0x00000800U
+#define PARENB		0x00001000U
+#define PARODD		0x00002000U
+#define HUPCL		0x00004000U
+#define CLOCAL		0x00008000U
+#define CIBAUD		0x001f0000U
 
 /* c_lflag bits */
-#define ISIG	0x00000080
-#define ICANON	0x00000100
-#define XCASE	0x00004000
-#define ECHO	0x00000008
-#define ECHOE	0x00000002
-#define ECHOK	0x00000004
-#define ECHONL	0x00000010
-#define NOFLSH	0x80000000
-#define TOSTOP	0x00400000
-#define ECHOCTL	0x00000040
-#define ECHOPRT	0x00000020
-#define ECHOKE	0x00000001
-#define FLUSHO	0x00800000
-#define PENDIN	0x20000000
-#define IEXTEN	0x00000400
-#define EXTPROC	0x10000000
+#define ISIG	0x00000080U
+#define ICANON	0x00000100U
+#define XCASE	0x00004000U
+#define ECHO	0x00000008U
+#define ECHOE	0x00000002U
+#define ECHOK	0x00000004U
+#define ECHONL	0x00000010U
+#define NOFLSH	0x80000000U
+#define TOSTOP	0x00400000U
+#define ECHOCTL	0x00000040U
+#define ECHOPRT	0x00000020U
+#define ECHOKE	0x00000001U
+#define FLUSHO	0x00800000U
+#define PENDIN	0x20000000U
+#define IEXTEN	0x00000400U
+#define EXTPROC	0x10000000U
 
 /* Values for the OPTIONAL_ACTIONS argument to `tcsetattr'.  */
 #define	TCSANOW		0
diff --git a/arch/mips/include/uapi/asm/termbits.h b/arch/mips/include/uapi/asm/termbits.h
index 1eb60903d6f0..35eb388a176e 100644
--- a/arch/mips/include/uapi/asm/termbits.h
+++ b/arch/mips/include/uapi/asm/termbits.h
@@ -78,96 +78,96 @@ struct ktermios {
 #define VEOL		17		/* End-of-line character [ICANON] */
 
 /* c_iflag bits */
-#define IUCLC	0x0200		/* Map upper case to lower case on input */
-#define IXON	0x0400		/* Enable start/stop output control */
-#define IXOFF	0x1000		/* Enable start/stop input control */
-#define IMAXBEL	0x2000		/* Ring bell when input queue is full */
-#define IUTF8	0x4000		/* Input is UTF-8 */
+#define IUCLC	0x0200U		/* Map upper case to lower case on input */
+#define IXON	0x0400U		/* Enable start/stop output control */
+#define IXOFF	0x1000U		/* Enable start/stop input control */
+#define IMAXBEL	0x2000U		/* Ring bell when input queue is full */
+#define IUTF8	0x4000U		/* Input is UTF-8 */
 
 /* c_oflag bits */
-#define OLCUC	0x00002		/* Map lower case to upper case on output */
-#define ONLCR	0x00004		/* Map NL to CR-NL on output */
-#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
+#define OLCUC	0x00002U	/* Map lower case to upper case on output */
+#define ONLCR	0x00004U	/* Map NL to CR-NL on output */
+#define NLDLY	0x00100U
+#define   NL0	0x00000U
+#define   NL1	0x00100U
+#define CRDLY	0x00600U
+#define   CR0	0x00000U
+#define   CR1	0x00200U
+#define   CR2	0x00400U
+#define   CR3	0x00600U
+#define TABDLY	0x01800U
+#define   TAB0	0x00000U
+#define   TAB1	0x00800U
+#define   TAB2	0x01000U
+#define   TAB3	0x01800U
+#define   XTABS	0x01800U
+#define BSDLY	0x02000U
+#define   BS0	0x00000U
+#define   BS1	0x02000U
+#define VTDLY	0x04000U
+#define   VT0	0x00000U
+#define   VT1	0x04000U
+#define FFDLY	0x08000U
+#define   FF0	0x00000U
+#define   FF1	0x08000U
 /*
 #define PAGEOUT ???
 #define WRAP	???
  */
 
 /* c_cflag bit meaning */
-#define CBAUD		0x0000100f
-#define CSIZE		0x00000030	/* Number of bits per byte (mask) */
-#define   CS5		0x00000000	/* 5 bits per byte */
-#define   CS6		0x00000010	/* 6 bits per byte */
-#define   CS7		0x00000020	/* 7 bits per byte */
-#define   CS8		0x00000030	/* 8 bits per byte */
-#define CSTOPB		0x00000040	/* Two stop bits instead of one */
-#define CREAD		0x00000080	/* Enable receiver */
-#define PARENB		0x00000100	/* Parity enable */
-#define PARODD		0x00000200	/* Odd parity instead of even */
-#define HUPCL		0x00000400	/* Hang up on last close */
-#define CLOCAL		0x00000800	/* Ignore modem status lines */
-#define CBAUDEX		0x00001000
-#define BOTHER		0x00001000
-#define     B57600	0x00001001
-#define    B115200	0x00001002
-#define    B230400	0x00001003
-#define    B460800	0x00001004
-#define    B500000	0x00001005
-#define    B576000	0x00001006
-#define    B921600	0x00001007
-#define   B1000000	0x00001008
-#define   B1152000	0x00001009
-#define   B1500000	0x0000100a
-#define   B2000000	0x0000100b
-#define   B2500000	0x0000100c
-#define   B3000000	0x0000100d
-#define   B3500000	0x0000100e
-#define   B4000000	0x0000100f
-#define CIBAUD		0x100f0000	/* input baud rate */
+#define CBAUD		0x0000100fU
+#define CSIZE		0x00000030U	/* Number of bits per byte (mask) */
+#define   CS5		0x00000000U	/* 5 bits per byte */
+#define   CS6		0x00000010U	/* 6 bits per byte */
+#define   CS7		0x00000020U	/* 7 bits per byte */
+#define   CS8		0x00000030U	/* 8 bits per byte */
+#define CSTOPB		0x00000040U	/* Two stop bits instead of one */
+#define CREAD		0x00000080U	/* Enable receiver */
+#define PARENB		0x00000100U	/* Parity enable */
+#define PARODD		0x00000200U	/* Odd parity instead of even */
+#define HUPCL		0x00000400U	/* Hang up on last close */
+#define CLOCAL		0x00000800U	/* Ignore modem status lines */
+#define CBAUDEX		0x00001000U
+#define BOTHER		0x00001000U
+#define     B57600	0x00001001U
+#define    B115200	0x00001002U
+#define    B230400	0x00001003U
+#define    B460800	0x00001004U
+#define    B500000	0x00001005U
+#define    B576000	0x00001006U
+#define    B921600	0x00001007U
+#define   B1000000	0x00001008U
+#define   B1152000	0x00001009U
+#define   B1500000	0x0000100aU
+#define   B2000000	0x0000100bU
+#define   B2500000	0x0000100cU
+#define   B3000000	0x0000100dU
+#define   B3500000	0x0000100eU
+#define   B4000000	0x0000100fU
+#define CIBAUD		0x100f0000U	/* input baud rate */
 
 /* c_lflag bits */
-#define ISIG	0x00001		/* Enable signals */
-#define ICANON	0x00002		/* Do erase and kill processing */
-#define XCASE	0x00004
-#define ECHO	0x00008		/* Enable echo */
-#define ECHOE	0x00010		/* Visual erase for ERASE */
-#define ECHOK	0x00020		/* Echo NL after KILL */
-#define ECHONL	0x00040		/* Echo NL even if ECHO is off */
-#define NOFLSH	0x00080		/* Disable flush after interrupt */
-#define IEXTEN	0x00100		/* Enable DISCARD and LNEXT */
-#define ECHOCTL	0x00200		/* Echo control characters as ^X */
-#define ECHOPRT	0x00400		/* Hardcopy visual erase */
-#define ECHOKE	0x00800		/* Visual erase for KILL */
-#define FLUSHO	0x02000
-#define PENDIN	0x04000		/* Retype pending input (state) */
-#define TOSTOP	0x08000		/* Send SIGTTOU for background output */
+#define ISIG	0x00001U	/* Enable signals */
+#define ICANON	0x00002U	/* Do erase and kill processing */
+#define XCASE	0x00004U
+#define ECHO	0x00008U	/* Enable echo */
+#define ECHOE	0x00010U	/* Visual erase for ERASE */
+#define ECHOK	0x00020U	/* Echo NL after KILL */
+#define ECHONL	0x00040U	/* Echo NL even if ECHO is off */
+#define NOFLSH	0x00080U	/* Disable flush after interrupt */
+#define IEXTEN	0x00100U	/* Enable DISCARD and LNEXT */
+#define ECHOCTL	0x00200U	/* Echo control characters as ^X */
+#define ECHOPRT	0x00400U	/* Hardcopy visual erase */
+#define ECHOKE	0x00800U	/* Visual erase for KILL */
+#define FLUSHO	0x02000U
+#define PENDIN	0x04000U	/* Retype pending input (state) */
+#define TOSTOP	0x08000U	/* Send SIGTTOU for background output */
 #define ITOSTOP	TOSTOP
-#define EXTPROC	0x10000		/* External processing on pty */
+#define EXTPROC	0x10000U	/* External processing on pty */
 
 /* ioctl (fd, TIOCSERGETLSR, &result) where result may be as below */
-#define TIOCSER_TEMT	0x01	/* Transmitter physically empty */
+#define TIOCSER_TEMT	0x01U	/* Transmitter physically empty */
 
 /* tcsetattr uses these */
 #define TCSANOW		TCSETS	/* Change immediately */
diff --git a/arch/parisc/include/uapi/asm/termbits.h b/arch/parisc/include/uapi/asm/termbits.h
index 3a8938d26fb4..d9090878c129 100644
--- a/arch/parisc/include/uapi/asm/termbits.h
+++ b/arch/parisc/include/uapi/asm/termbits.h
@@ -58,88 +58,88 @@ struct ktermios {
 #define VEOL2		16
 
 /* c_iflag bits */
-#define IUCLC	0x0200
-#define IXON	0x0400
-#define IXOFF	0x1000
-#define IMAXBEL	0x4000
-#define IUTF8	0x8000
+#define IUCLC	0x0200U
+#define IXON	0x0400U
+#define IXOFF	0x1000U
+#define IMAXBEL	0x4000U
+#define IUTF8	0x8000U
 
 /* 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
+#define OLCUC	0x00002U
+#define ONLCR	0x00004U
+#define NLDLY	0x00100U
+#define   NL0	0x00000U
+#define   NL1	0x00100U
+#define CRDLY	0x00600U
+#define   CR0	0x00000U
+#define   CR1	0x00200U
+#define   CR2	0x00400U
+#define   CR3	0x00600U
+#define TABDLY	0x01800U
+#define   TAB0	0x00000U
+#define   TAB1	0x00800U
+#define   TAB2	0x01000U
+#define   TAB3	0x01800U
+#define   XTABS	0x01800U
+#define BSDLY	0x02000U
+#define   BS0	0x00000U
+#define   BS1	0x02000U
+#define VTDLY	0x04000U
+#define   VT0	0x00000U
+#define   VT1	0x04000U
+#define FFDLY	0x08000U
+#define   FF0	0x00000U
+#define   FF1	0x08000U
 
 /* c_cflag bit meaning */
-#define CBAUD		0x0000100f
-#define CSIZE		0x00000030
-#define   CS5		0x00000000
-#define   CS6		0x00000010
-#define   CS7		0x00000020
-#define   CS8		0x00000030
-#define CSTOPB		0x00000040
-#define CREAD		0x00000080
-#define PARENB		0x00000100
-#define PARODD		0x00000200
-#define HUPCL		0x00000400
-#define CLOCAL		0x00000800
-#define CBAUDEX		0x00001000
-#define BOTHER		0x00001000
-#define     B57600	0x00001001
-#define    B115200	0x00001002
-#define    B230400	0x00001003
-#define    B460800	0x00001004
-#define    B500000	0x00001005
-#define    B576000	0x00001006
-#define    B921600	0x00001007
-#define   B1000000	0x00001008
-#define   B1152000	0x00001009
-#define   B1500000	0x0000100a
-#define   B2000000	0x0000100b
-#define   B2500000	0x0000100c
-#define   B3000000	0x0000100d
-#define   B3500000	0x0000100e
-#define   B4000000	0x0000100f
-#define CIBAUD		0x100f0000		/* input baud rate */
+#define CBAUD		0x0000100fU
+#define CSIZE		0x00000030U
+#define   CS5		0x00000000U
+#define   CS6		0x00000010U
+#define   CS7		0x00000020U
+#define   CS8		0x00000030U
+#define CSTOPB		0x00000040U
+#define CREAD		0x00000080U
+#define PARENB		0x00000100U
+#define PARODD		0x00000200U
+#define HUPCL		0x00000400U
+#define CLOCAL		0x00000800U
+#define CBAUDEX		0x00001000U
+#define BOTHER		0x00001000U
+#define     B57600	0x00001001U
+#define    B115200	0x00001002U
+#define    B230400	0x00001003U
+#define    B460800	0x00001004U
+#define    B500000	0x00001005U
+#define    B576000	0x00001006U
+#define    B921600	0x00001007U
+#define   B1000000	0x00001008U
+#define   B1152000	0x00001009U
+#define   B1500000	0x0000100aU
+#define   B2000000	0x0000100bU
+#define   B2500000	0x0000100cU
+#define   B3000000	0x0000100dU
+#define   B3500000	0x0000100eU
+#define   B4000000	0x0000100fU
+#define CIBAUD		0x100f0000U		/* input baud rate */
 
 /* 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
+#define ISIG	0x00001U
+#define ICANON	0x00002U
+#define XCASE	0x00004U
+#define ECHO	0x00008U
+#define ECHOE	0x00010U
+#define ECHOK	0x00020U
+#define ECHONL	0x00040U
+#define NOFLSH	0x00080U
+#define TOSTOP	0x00100U
+#define ECHOCTL	0x00200U
+#define ECHOPRT	0x00400U
+#define ECHOKE	0x00800U
+#define FLUSHO	0x01000U
+#define PENDIN	0x04000U
+#define IEXTEN	0x08000U
+#define EXTPROC	0x10000U
 
 /* tcsetattr uses these */
 #define	TCSANOW		0
diff --git a/arch/powerpc/include/uapi/asm/termbits.h b/arch/powerpc/include/uapi/asm/termbits.h
index 21dc86dcb2f1..ae3c2ed3dae8 100644
--- a/arch/powerpc/include/uapi/asm/termbits.h
+++ b/arch/powerpc/include/uapi/asm/termbits.h
@@ -64,90 +64,90 @@ struct ktermios {
 #define VDISCARD	16
 
 /* c_iflag bits */
-#define IXON	0x0200
-#define IXOFF	0x0400
-#define IUCLC	0x1000
-#define IMAXBEL	0x2000
-#define IUTF8	0x4000
+#define IXON	0x0200U
+#define IXOFF	0x0400U
+#define IUCLC	0x1000U
+#define IMAXBEL	0x2000U
+#define IUTF8	0x4000U
 
 /* c_oflag bits */
-#define ONLCR	0x00002
-#define OLCUC	0x00004
-#define NLDLY	0x00300
-#define   NL0	0x00000
-#define   NL1	0x00100
-#define   NL2	0x00200
-#define   NL3	0x00300
-#define TABDLY	0x00c00
-#define   TAB0	0x00000
-#define   TAB1	0x00400
-#define   TAB2	0x00800
-#define   TAB3	0x00c00
-#define   XTABS	0x00c00		/* required by POSIX to == TAB3 */
-#define CRDLY	0x03000
-#define   CR0	0x00000
-#define   CR1	0x01000
-#define   CR2	0x02000
-#define   CR3	0x03000
-#define FFDLY	0x04000
-#define   FF0	0x00000
-#define   FF1	0x04000
-#define BSDLY	0x08000
-#define   BS0	0x00000
-#define   BS1	0x08000
-#define VTDLY	0x10000
-#define   VT0	0x00000
-#define   VT1	0x10000
+#define ONLCR	0x00002U
+#define OLCUC	0x00004U
+#define NLDLY	0x00300U
+#define   NL0	0x00000U
+#define   NL1	0x00100U
+#define   NL2	0x00200U
+#define   NL3	0x00300U
+#define TABDLY	0x00c00U
+#define   TAB0	0x00000U
+#define   TAB1	0x00400U
+#define   TAB2	0x00800U
+#define   TAB3	0x00c00U
+#define   XTABS	0x00c00U	/* required by POSIX to == TAB3 */
+#define CRDLY	0x03000U
+#define   CR0	0x00000U
+#define   CR1	0x01000U
+#define   CR2	0x02000U
+#define   CR3	0x03000U
+#define FFDLY	0x04000U
+#define   FF0	0x00000U
+#define   FF1	0x04000U
+#define BSDLY	0x08000U
+#define   BS0	0x00000U
+#define   BS1	0x08000U
+#define VTDLY	0x10000U
+#define   VT0	0x00000U
+#define   VT1	0x10000U
 
 /* c_cflag bit meaning */
-#define CBAUD		0x000000ff
-#define CBAUDEX		0x00000000
-#define BOTHER		0x0000001f
-#define    B57600	0x00000010
-#define   B115200	0x00000011
-#define   B230400	0x00000012
-#define   B460800	0x00000013
-#define   B500000	0x00000014
-#define   B576000	0x00000015
-#define   B921600	0x00000016
-#define  B1000000	0x00000017
-#define  B1152000	0x00000018
-#define  B1500000	0x00000019
-#define  B2000000	0x0000001a
-#define  B2500000	0x0000001b
-#define  B3000000	0x0000001c
-#define  B3500000	0x0000001d
-#define  B4000000	0x0000001e
-#define CSIZE		0x00000300
-#define   CS5		0x00000000
-#define   CS6		0x00000100
-#define   CS7		0x00000200
-#define   CS8		0x00000300
-#define CSTOPB		0x00000400
-#define CREAD		0x00000800
-#define PARENB		0x00001000
-#define PARODD		0x00002000
-#define HUPCL		0x00004000
-#define CLOCAL		0x00008000
-#define CIBAUD		0x00ff0000
+#define CBAUD		0x000000ffU
+#define CBAUDEX		0x00000000U
+#define BOTHER		0x0000001fU
+#define    B57600	0x00000010U
+#define   B115200	0x00000011U
+#define   B230400	0x00000012U
+#define   B460800	0x00000013U
+#define   B500000	0x00000014U
+#define   B576000	0x00000015U
+#define   B921600	0x00000016U
+#define  B1000000	0x00000017U
+#define  B1152000	0x00000018U
+#define  B1500000	0x00000019U
+#define  B2000000	0x0000001aU
+#define  B2500000	0x0000001bU
+#define  B3000000	0x0000001cU
+#define  B3500000	0x0000001dU
+#define  B4000000	0x0000001eU
+#define CSIZE		0x00000300U
+#define   CS5		0x00000000U
+#define   CS6		0x00000100U
+#define   CS7		0x00000200U
+#define   CS8		0x00000300U
+#define CSTOPB		0x00000400U
+#define CREAD		0x00000800U
+#define PARENB		0x00001000U
+#define PARODD		0x00002000U
+#define HUPCL		0x00004000U
+#define CLOCAL		0x00008000U
+#define CIBAUD		0x00ff0000U
 
 /* c_lflag bits */
-#define ISIG	0x00000080
-#define ICANON	0x00000100
-#define XCASE	0x00004000
-#define ECHO	0x00000008
-#define ECHOE	0x00000002
-#define ECHOK	0x00000004
-#define ECHONL	0x00000010
-#define NOFLSH	0x80000000
-#define TOSTOP	0x00400000
-#define ECHOCTL	0x00000040
-#define ECHOPRT	0x00000020
-#define ECHOKE	0x00000001
-#define FLUSHO	0x00800000
-#define PENDIN	0x20000000
-#define IEXTEN	0x00000400
-#define EXTPROC	0x10000000
+#define ISIG	0x00000080U
+#define ICANON	0x00000100U
+#define XCASE	0x00004000U
+#define ECHO	0x00000008U
+#define ECHOE	0x00000002U
+#define ECHOK	0x00000004U
+#define ECHONL	0x00000010U
+#define NOFLSH	0x80000000U
+#define TOSTOP	0x00400000U
+#define ECHOCTL	0x00000040U
+#define ECHOPRT	0x00000020U
+#define ECHOKE	0x00000001U
+#define FLUSHO	0x00800000U
+#define PENDIN	0x20000000U
+#define IEXTEN	0x00000400U
+#define EXTPROC	0x10000000U
 
 /* Values for the OPTIONAL_ACTIONS argument to `tcsetattr'.  */
 #define	TCSANOW		0
diff --git a/arch/sparc/include/uapi/asm/termbits.h b/arch/sparc/include/uapi/asm/termbits.h
index 0da2b1adc0f5..5cd0b28b16dc 100644
--- a/arch/sparc/include/uapi/asm/termbits.h
+++ b/arch/sparc/include/uapi/asm/termbits.h
@@ -75,121 +75,121 @@ struct ktermios {
 #endif
 
 /* c_iflag bits */
-#define IUCLC	0x0200
-#define IXON	0x0400
-#define IXOFF	0x1000
-#define IMAXBEL	0x2000
-#define IUTF8   0x4000
+#define IUCLC	0x0200U
+#define IXON	0x0400U
+#define IXOFF	0x1000U
+#define IMAXBEL	0x2000U
+#define IUTF8   0x4000U
 
 /* 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
-#define PAGEOUT 0x10000			/* SUNOS specific */
-#define WRAP    0x20000			/* SUNOS specific */
+#define OLCUC	0x00002U
+#define ONLCR	0x00004U
+#define NLDLY	0x00100U
+#define   NL0	0x00000U
+#define   NL1	0x00100U
+#define CRDLY	0x00600U
+#define   CR0	0x00000U
+#define   CR1	0x00200U
+#define   CR2	0x00400U
+#define   CR3	0x00600U
+#define TABDLY	0x01800U
+#define   TAB0	0x00000U
+#define   TAB1	0x00800U
+#define   TAB2	0x01000U
+#define   TAB3	0x01800U
+#define   XTABS	0x01800U
+#define BSDLY	0x02000U
+#define   BS0	0x00000U
+#define   BS1	0x02000U
+#define VTDLY	0x04000U
+#define   VT0	0x00000U
+#define   VT1	0x04000U
+#define FFDLY	0x08000U
+#define   FF0	0x00000U
+#define   FF1	0x08000U
+#define PAGEOUT 0x10000U		/* SUNOS specific */
+#define WRAP    0x20000U		/* SUNOS specific */
 
 /* c_cflag bit meaning */
-#define CBAUD		0x0000100f
-#define CSIZE		0x00000030
-#define   CS5		0x00000000
-#define   CS6		0x00000010
-#define   CS7		0x00000020
-#define   CS8		0x00000030
-#define CSTOPB		0x00000040
-#define CREAD		0x00000080
-#define PARENB		0x00000100
-#define PARODD		0x00000200
-#define HUPCL		0x00000400
-#define CLOCAL		0x00000800
-#define CBAUDEX		0x00001000
+#define CBAUD		0x0000100fU
+#define CSIZE		0x00000030U
+#define   CS5		0x00000000U
+#define   CS6		0x00000010U
+#define   CS7		0x00000020U
+#define   CS8		0x00000030U
+#define CSTOPB		0x00000040U
+#define CREAD		0x00000080U
+#define PARENB		0x00000100U
+#define PARODD		0x00000200U
+#define HUPCL		0x00000400U
+#define CLOCAL		0x00000800U
+#define CBAUDEX		0x00001000U
 /* We'll never see these speeds with the Zilogs, but for completeness... */
-#define BOTHER		0x00001000
-#define     B57600	0x00001001
-#define    B115200	0x00001002
-#define    B230400	0x00001003
-#define    B460800	0x00001004
+#define BOTHER		0x00001000U
+#define     B57600	0x00001001U
+#define    B115200	0x00001002U
+#define    B230400	0x00001003U
+#define    B460800	0x00001004U
 /* This is what we can do with the Zilogs. */
-#define     B76800	0x00001005
+#define     B76800	0x00001005U
 /* This is what we can do with the SAB82532. */
-#define    B153600	0x00001006
-#define    B307200	0x00001007
-#define    B614400	0x00001008
-#define    B921600	0x00001009
+#define    B153600	0x00001006U
+#define    B307200	0x00001007U
+#define    B614400	0x00001008U
+#define    B921600	0x00001009U
 /* And these are the rest... */
-#define    B500000	0x0000100a
-#define    B576000	0x0000100b
-#define   B1000000	0x0000100c
-#define   B1152000	0x0000100d
-#define   B1500000	0x0000100e
-#define   B2000000	0x0000100f
+#define    B500000	0x0000100aU
+#define    B576000	0x0000100bU
+#define   B1000000	0x0000100cU
+#define   B1152000	0x0000100dU
+#define   B1500000	0x0000100eU
+#define   B2000000	0x0000100fU
 /* These have totally bogus values and nobody uses them
    so far. Later on we'd have to use say 0x10000x and
    adjust CBAUD constant and drivers accordingly.
-#define   B2500000	0x00001010
-#define   B3000000	0x00001011
-#define   B3500000	0x00001012
-#define   B4000000	0x00001013 */
-#define CIBAUD		0x100f0000	/* input baud rate (not used) */
+#define   B2500000	0x00001010U
+#define   B3000000	0x00001011U
+#define   B3500000	0x00001012U
+#define   B4000000	0x00001013U */
+#define CIBAUD		0x100f0000U	/* input baud rate (not used) */
 
 /* c_lflag bits */
-#define ISIG	0x00000001
-#define ICANON	0x00000002
-#define XCASE	0x00000004
-#define ECHO	0x00000008
-#define ECHOE	0x00000010
-#define ECHOK	0x00000020
-#define ECHONL	0x00000040
-#define NOFLSH	0x00000080
-#define TOSTOP	0x00000100
-#define ECHOCTL	0x00000200
-#define ECHOPRT	0x00000400
-#define ECHOKE	0x00000800
-#define DEFECHO 0x00001000		/* SUNOS thing, what is it? */
-#define FLUSHO	0x00002000
-#define PENDIN	0x00004000
-#define IEXTEN	0x00008000
-#define EXTPROC	0x00010000
+#define ISIG	0x00000001U
+#define ICANON	0x00000002U
+#define XCASE	0x00000004U
+#define ECHO	0x00000008U
+#define ECHOE	0x00000010U
+#define ECHOK	0x00000020U
+#define ECHONL	0x00000040U
+#define NOFLSH	0x00000080U
+#define TOSTOP	0x00000100U
+#define ECHOCTL	0x00000200U
+#define ECHOPRT	0x00000400U
+#define ECHOKE	0x00000800U
+#define DEFECHO 0x00001000U		/* SUNOS thing, what is it? */
+#define FLUSHO	0x00002000U
+#define PENDIN	0x00004000U
+#define IEXTEN	0x00008000U
+#define EXTPROC	0x00010000U
 
 /* modem lines */
-#define TIOCM_LE	0x001
-#define TIOCM_DTR	0x002
-#define TIOCM_RTS	0x004
-#define TIOCM_ST	0x008
-#define TIOCM_SR	0x010
-#define TIOCM_CTS	0x020
-#define TIOCM_CAR	0x040
-#define TIOCM_RNG	0x080
-#define TIOCM_DSR	0x100
+#define TIOCM_LE	0x001U
+#define TIOCM_DTR	0x002U
+#define TIOCM_RTS	0x004U
+#define TIOCM_ST	0x008U
+#define TIOCM_SR	0x010U
+#define TIOCM_CTS	0x020U
+#define TIOCM_CAR	0x040U
+#define TIOCM_RNG	0x080U
+#define TIOCM_DSR	0x100U
 #define TIOCM_CD	TIOCM_CAR
 #define TIOCM_RI	TIOCM_RNG
-#define TIOCM_OUT1	0x2000
-#define TIOCM_OUT2	0x4000
-#define TIOCM_LOOP	0x8000
+#define TIOCM_OUT1	0x2000U
+#define TIOCM_OUT2	0x4000U
+#define TIOCM_LOOP	0x8000U
 
 /* ioctl (fd, TIOCSERGETLSR, &result) where result may be as below */
-#define TIOCSER_TEMT    0x01	/* Transmitter physically empty */
+#define TIOCSER_TEMT    0x01U	/* Transmitter physically empty */
 
 /* tcsetattr uses these */
 #define TCSANOW		0
diff --git a/include/uapi/asm-generic/termbits.h b/include/uapi/asm-generic/termbits.h
index 890ef29053e2..10e01c77d2a6 100644
--- a/include/uapi/asm-generic/termbits.h
+++ b/include/uapi/asm-generic/termbits.h
@@ -58,88 +58,88 @@ struct ktermios {
 #define VEOL2		16
 
 /* c_iflag bits */
-#define IUCLC	0x0200
-#define IXON	0x0400
-#define IXOFF	0x1000
-#define IMAXBEL	0x2000
-#define IUTF8	0x4000
+#define IUCLC	0x0200U
+#define IXON	0x0400U
+#define IXOFF	0x1000U
+#define IMAXBEL	0x2000U
+#define IUTF8	0x4000U
 
 /* 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
+#define OLCUC	0x00002U
+#define ONLCR	0x00004U
+#define NLDLY	0x00100U
+#define   NL0	0x00000U
+#define   NL1	0x00100U
+#define CRDLY	0x00600U
+#define   CR0	0x00000U
+#define   CR1	0x00200U
+#define   CR2	0x00400U
+#define   CR3	0x00600U
+#define TABDLY	0x01800U
+#define   TAB0	0x00000U
+#define   TAB1	0x00800U
+#define   TAB2	0x01000U
+#define   TAB3	0x01800U
+#define   XTABS	0x01800U
+#define BSDLY	0x02000U
+#define   BS0	0x00000U
+#define   BS1	0x02000U
+#define VTDLY	0x04000U
+#define   VT0	0x00000U
+#define   VT1	0x04000U
+#define FFDLY	0x08000U
+#define   FF0	0x00000U
+#define   FF1	0x08000U
 
 /* c_cflag bit meaning */
-#define CBAUD		0x0000100f
-#define CSIZE		0x00000030
-#define   CS5		0x00000000
-#define   CS6		0x00000010
-#define   CS7		0x00000020
-#define   CS8		0x00000030
-#define CSTOPB		0x00000040
-#define CREAD		0x00000080
-#define PARENB		0x00000100
-#define PARODD		0x00000200
-#define HUPCL		0x00000400
-#define CLOCAL		0x00000800
-#define CBAUDEX		0x00001000
-#define BOTHER		0x00001000
-#define     B57600	0x00001001
-#define    B115200	0x00001002
-#define    B230400	0x00001003
-#define    B460800	0x00001004
-#define    B500000	0x00001005
-#define    B576000	0x00001006
-#define    B921600	0x00001007
-#define   B1000000	0x00001008
-#define   B1152000	0x00001009
-#define   B1500000	0x0000100a
-#define   B2000000	0x0000100b
-#define   B2500000	0x0000100c
-#define   B3000000	0x0000100d
-#define   B3500000	0x0000100e
-#define   B4000000	0x0000100f
-#define CIBAUD		0x100f0000	/* input baud rate */
+#define CBAUD		0x0000100fU
+#define CSIZE		0x00000030U
+#define   CS5		0x00000000U
+#define   CS6		0x00000010U
+#define   CS7		0x00000020U
+#define   CS8		0x00000030U
+#define CSTOPB		0x00000040U
+#define CREAD		0x00000080U
+#define PARENB		0x00000100U
+#define PARODD		0x00000200U
+#define HUPCL		0x00000400U
+#define CLOCAL		0x00000800U
+#define CBAUDEX		0x00001000U
+#define BOTHER		0x00001000U
+#define     B57600	0x00001001U
+#define    B115200	0x00001002U
+#define    B230400	0x00001003U
+#define    B460800	0x00001004U
+#define    B500000	0x00001005U
+#define    B576000	0x00001006U
+#define    B921600	0x00001007U
+#define   B1000000	0x00001008U
+#define   B1152000	0x00001009U
+#define   B1500000	0x0000100aU
+#define   B2000000	0x0000100bU
+#define   B2500000	0x0000100cU
+#define   B3000000	0x0000100dU
+#define   B3500000	0x0000100eU
+#define   B4000000	0x0000100fU
+#define CIBAUD		0x100f0000U	/* input baud rate */
 
 /* 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
+#define ISIG	0x00001U
+#define ICANON	0x00002U
+#define XCASE	0x00004U
+#define ECHO	0x00008U
+#define ECHOE	0x00010U
+#define ECHOK	0x00020U
+#define ECHONL	0x00040U
+#define NOFLSH	0x00080U
+#define TOSTOP	0x00100U
+#define ECHOCTL	0x00200U
+#define ECHOPRT	0x00400U
+#define ECHOKE	0x00800U
+#define FLUSHO	0x01000U
+#define PENDIN	0x04000U
+#define IEXTEN	0x08000U
+#define EXTPROC	0x10000U
 
 /* tcsetattr uses these */
 #define	TCSANOW		0

base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
-- 
2.45.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields

[Greg KH]

On Wed, Jun 12, 2024 at 03:16:58PM +0200, Alejandro Colomar wrote:
> Constants that are to be used in bitwise operations should be unsigned,
> or a user could easily trigger Undefined Behavior.

Wait, do we really have such broken compilers out there?  If so, how has
no one hit this before?

> Also, the types where these constants are to be assigned are unsigned,
> so this makes it more consistent.

Fair enough.

> alx@debian:/usr/include$ grepc -tt termios asm-generic/
> 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 */
> };
> alx@debian:/usr/include$ grepc -tt tcflag_t asm-generic/
> asm-generic/termbits.h:typedef unsigned int	tcflag_t;
> alx@debian:/usr/include$ grepc -tt cc_t asm-generic/
> asm-generic/termbits-common.h:typedef unsigned char	cc_t;
> alx@debian:/usr/include$ grepc -tt speed_t asm-generic/
> asm-generic/termbits-common.h:typedef unsigned int	speed_t;
> 
> Link: <https://lore.kernel.org/linux-api/2024061222-scuttle-expanse-6438@gregkh/T/>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Palmer Dabbelt <palmer@rivosinc.com>
> Cc: <linux-api@vger.kernel.org>
> Cc: <libc-alpha@sourceware.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
> 
> Hi Greg,
> 
> On Wed, Jun 12, 2024 at 02:22:37PM GMT, Greg KH wrote:
> > Have a proposed patch that you feel would resolve this?
> >
> > thanks,
> >
> > greg k-h
> 
> Here it is.  :)
> 
> For reviewing it, I suggest using '--word-diff-regex=.'.
> 
> I compiled the kernel, and it seems ok; didn't test more than that.

With this change, can the glibc versions then be dropped to just rely on
these instead?

thanks,

greg k-h

Re: [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

Hi Greg,

On Wed, Jun 12, 2024 at 03:35:20PM GMT, Greg KH wrote:
> On Wed, Jun 12, 2024 at 03:16:58PM +0200, Alejandro Colomar wrote:
> > Constants that are to be used in bitwise operations should be unsigned,
> > or a user could easily trigger Undefined Behavior.
> 
> Wait, do we really have such broken compilers out there?

I meant this as a generic statement that signed integers on bitwise ops
are bad, not as a specific statement that these values would trigger UB.

I expect that these specific values and the operations done on them
probably don't trigger UB, since the shifts are done by a controlled
amount, and there are justa few operations done on them.

For example, a left shift where a set bit overflows the type (e.g.,
1<<32), causes UB.

The reason why it's better to avoid this at all even if we know these
values work fine, is that programs using <asm/termbits.h> would need to
disable those compiler warnings, which could silence warnings on other
code which might be broken.

TL;DR: The kernel isn't broken, but improving this would allow users to
enable stricter warnings, which is a good thing.

> With this change, can the glibc versions then be dropped to just rely on
> these instead?

I don't know.  glibc is CCd, so they can answer that.

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields

[Greg KH]

On Wed, Jun 12, 2024 at 04:00:18PM +0200, Alejandro Colomar wrote:
> Hi Greg,
> 
> On Wed, Jun 12, 2024 at 03:35:20PM GMT, Greg KH wrote:
> > On Wed, Jun 12, 2024 at 03:16:58PM +0200, Alejandro Colomar wrote:
> > > Constants that are to be used in bitwise operations should be unsigned,
> > > or a user could easily trigger Undefined Behavior.
> > 
> > Wait, do we really have such broken compilers out there?
> 
> I meant this as a generic statement that signed integers on bitwise ops
> are bad, not as a specific statement that these values would trigger UB.
> 
> I expect that these specific values and the operations done on them
> probably don't trigger UB, since the shifts are done by a controlled
> amount, and there are justa few operations done on them.

These, for the most part, are NOT used as shifts.

> For example, a left shift where a set bit overflows the type (e.g.,
> 1<<32), causes UB.

Sure, but that's not in play here.

> The reason why it's better to avoid this at all even if we know these
> values work fine, is that programs using <asm/termbits.h> would need to
> disable those compiler warnings, which could silence warnings on other
> code which might be broken.

But again, you aren't using these as bit shifts, they are bit masks, or
values, only.

> TL;DR: The kernel isn't broken, but improving this would allow users to
> enable stricter warnings, which is a good thing.

Enable it where?

thanks,

greg k-h

Re: termios constants should be unsigned

[Paul Eggert]

On 2024-06-12 05:16, Alejandro Colomar wrote:
> tcgets.c:53:24:
>   error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka
>   'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]

This is a bug in Clang not glibc, and if you're worried about it I 
suggest sending a bug report to the Clang folks about the false positive.

Even GCC's -Wsign-conversion, which is at least smart enough to not warn 
about benign conversions like that, is too often so chatty that it's 
best avoided.

A lot of this stuff is pedanticism that dates back to the bad old days 
when the C standard allowed ones' complement and signed magnitude 
representations of signed integers. Although it can be amusing to worry 
about that possibility (I know I've done it) it's never been a practical 
worry, and even the motivation of pedanticism is going away now that C23 
requires two's complement.

Re: [PATCH] uapi/asm/termbits: Use the U integer suffix for bit fields

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

Hi Greg,

On Wed, Jun 12, 2024 at 04:21:25PM GMT, Greg KH wrote:
> On Wed, Jun 12, 2024 at 04:00:18PM +0200, Alejandro Colomar wrote:
> > I expect that these specific values and the operations done on them
> > probably don't trigger UB, since the shifts are done by a controlled
> > amount, and there are justa few operations done on them.
> 
> These, for the most part, are NOT used as shifts.

Quoting the EXAMPLES section in the manual page:

	tio.c_cflag &= ~(CBAUD << IBSHIFT);

(And yeah, that shift is presumably controlled, so that it doesn't
overflow, which is why I mean these are presumably just fine.)

> > TL;DR: The kernel isn't broken, but improving this would allow users to
> > enable stricter warnings, which is a good thing.
> 
> Enable it where?

I meant in user space programs that use termbits stuff.  (That this may
also allow the kernel to eventually have stricter warnings, I don't
know.  It might help.  But mostly meant it for user space.)

So, if I have a user-space program (or more likely a library) which
wraps these ioctls, I'd prefer to be able to enable the warnings I
reported, to preclude any mistakes in my code.  That would need the
constants to be unsigned, to avoid false negatives.

Cheers,
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: termios constants should be unsigned

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

Hi Paul,

On Wed, Jun 12, 2024 at 07:55:14AM GMT, Paul Eggert wrote:
> On 2024-06-12 05:16, Alejandro Colomar wrote:
> > tcgets.c:53:24:
> >   error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka
> >   'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
> 
> This is a bug in Clang not glibc, and if you're worried about it I suggest
> sending a bug report to the Clang folks about the false positive.
> 
> Even GCC's -Wsign-conversion, which is at least smart enough to not warn
> about benign conversions like that, is too often so chatty that it's best
> avoided.
> 
> A lot of this stuff is pedanticism that dates back to the bad old days when
> the C standard allowed ones' complement and signed magnitude representations
> of signed integers. Although it can be amusing to worry about that
> possibility (I know I've done it) it's never been a practical worry, and
> even the motivation of pedanticism is going away now that C23 requires two's
> complement.

I know; I think I have -Weverything enabled in that run, which is known
for its pedanticity.  I usually disable it when it triggers a warning,
since they are usually nonsense.  But in this case, adding U is a net
improvement, without downsides (or I can't see them).

So, while the kernel and glibc are just fine with this implicit
conversion, they would be equally fine and even better without the
conversion.  Not a bug, but rather a slight improvement.

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: termios constants should be unsigned

[enh]

On Wed, Jun 12, 2024 at 12:29 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> Hi Paul,
>
> On Wed, Jun 12, 2024 at 07:55:14AM GMT, Paul Eggert wrote:
> > On 2024-06-12 05:16, Alejandro Colomar wrote:
> > > tcgets.c:53:24:
> > >   error: implicit conversion changes signedness: 'int' to 'tcflag_t' (aka
> > >   'unsigned int') [clang-diagnostic-sign-conversion,-warnings-as-errors]
> >
> > This is a bug in Clang not glibc, and if you're worried about it I suggest
> > sending a bug report to the Clang folks about the false positive.
> >
> > Even GCC's -Wsign-conversion, which is at least smart enough to not warn
> > about benign conversions like that, is too often so chatty that it's best
> > avoided.
> >
> > A lot of this stuff is pedanticism that dates back to the bad old days when
> > the C standard allowed ones' complement and signed magnitude representations
> > of signed integers. Although it can be amusing to worry about that
> > possibility (I know I've done it) it's never been a practical worry, and
> > even the motivation of pedanticism is going away now that C23 requires two's
> > complement.
>
> I know; I think I have -Weverything enabled in that run, which is known
> for its pedanticity.  I usually disable it when it triggers a warning,
> since they are usually nonsense.  But in this case, adding U is a net
> improvement, without downsides (or I can't see them).

well, any change like this is a potential source incompatibility ... i
hacked these changes into AOSP, and it did break one bit of existing
code that was already working around the sign differences --- this
warning was enabled but the code had a cast to make the _other_ side
of the comparison signed (rather than make this side of the comparison
unsigned).

Android's libc [bionic] uses the uapi headers directly, so we would be
affected, but to be clear --- i'm fine with this if the consensus is
to go this way.

(but, yeah, i'm with the "how about we fix the language and compiler
rather than all the extant code?" sentiment from Paul Eggert.)

> So, while the kernel and glibc are just fine with this implicit
> conversion, they would be equally fine and even better without the
> conversion.  Not a bug, but rather a slight improvement.
>
> Have a lovely day!
> Alex
>
> --
> <https://www.alejandro-colomar.es/>

Re: termios constants should be unsigned

[Paul Eggert]

On 2024-06-12 09:28, Alejandro Colomar wrote:
> adding U is a net
> improvement, without downsides (or I can't see them)

Adding U can change the generated code and thus cause behavior change in 
programs (admittedly not well-written ones). It could also create new 
false positives in well-written programs.

If adding U fixed real bugs then of course we should do it. Here, though....

Re: termios constants should be unsigned

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]

Hi Elliott, Paul,

On Wed, Jun 12, 2024 at 01:47:03PM GMT, enh wrote:
> well, any change like this is a potential source incompatibility ... i
> hacked these changes into AOSP, and it did break one bit of existing
> code that was already working around the sign differences --- this
> warning was enabled but the code had a cast to make the _other_ side
> of the comparison signed (rather than make this side of the comparison
> unsigned).
> 
> Android's libc [bionic] uses the uapi headers directly, so we would be
> affected, but to be clear --- i'm fine with this if the consensus is
> to go this way.

Hmmm; I see.  I guess for this already-existent case we can just live
with it.  Then I'll just ask to consider using unsigned constants for
new stuff that is a bit pattern and not just a number.

Let's drop the patch.

Have a lovely day!
Alex

> (but, yeah, i'm with the "how about we fix the language and compiler
> rather than all the extant code?" sentiment from Paul Eggert.)

Hmmm, can agree to that.  At least if we don't continue writing bad code
for the new one.  :)

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: termios constants should be unsigned

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

On Wed, Jun 12, 2024 at 01:47:03PM GMT, enh wrote:
> hacked these changes into AOSP, and it did break one bit of existing
> code that was already working around the sign differences --- this
> warning was enabled but the code had a cast to make the _other_ side
> of the comparison signed (rather than make this side of the comparison
> unsigned).

BTW, that seems to be a bogus way to workaround this; the cast should
have been on the other side.  I'd say whoever maintains that code should
probably fix that to use unsigned types.  These constants are meant to
be 'tcflag_t', so a cast should be to that type, or the type of the
other side of the comparison, but casting to 'int' just for silencing a
waring seems nuts.

This makes me wonder if breaking _those_ users could be a good thing...

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: termios constants should be unsigned

[enh]

On Wed, Jun 12, 2024 at 3:01 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> On Wed, Jun 12, 2024 at 01:47:03PM GMT, enh wrote:
> > hacked these changes into AOSP, and it did break one bit of existing
> > code that was already working around the sign differences --- this
> > warning was enabled but the code had a cast to make the _other_ side
> > of the comparison signed (rather than make this side of the comparison
> > unsigned).
>
> BTW, that seems to be a bogus way to workaround this; the cast should
> have been on the other side.  I'd say whoever maintains that code should
> probably fix that to use unsigned types.

indeed. i've already sent out such a change :-)

>  These constants are meant to
> be 'tcflag_t', so a cast should be to that type, or the type of the
> other side of the comparison, but casting to 'int' just for silencing a
> waring seems nuts.

i suspect the reasoning was one of readability --- keeping the [short]
constants legible at the cost of making the expression slightly
longer.

> This makes me wonder if breaking _those_ users could be a good thing...

like Paul Eggert said somewhere else today --- only if we're finding
real bugs. and so far we're not.

it's like the warn_unused_result argument. a purist would argue that
every function should have that annotation, because you should always
check for errors, and if you're not already doing so, your code is
already broken. whereas a pragmatist would argue that most people are
just going to add the "shut up, compiler" cast (or disable the warning
entirely) if their already-working code suddenly starts spamming
warnings next time they build it.

while my bar for that might not be as high as my bar for ABI breakage,
my source compatibility bar is still pretty high. it would be almost
unethical of me to make app developers do random busywork. i have to
be pretty confident (as with, say, "you just passed an fd > 1024 to an
fd_set function/macro and thus corrupted memory") that their code is
_definitely_ wrong. (and even there, that's going to have to be a
runtime check!)

> --
> <https://www.alejandro-colomar.es/>

Re: termios constants should be unsigned

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]

Hi Elliott,

On Wed, Jun 12, 2024 at 05:54:43PM GMT, enh wrote:
> > BTW, that seems to be a bogus way to workaround this; the cast should
> > have been on the other side.  I'd say whoever maintains that code should
> > probably fix that to use unsigned types.
> 
> indeed. i've already sent out such a change :-)
> 
> >  These constants are meant to
> > be 'tcflag_t', so a cast should be to that type, or the type of the
> > other side of the comparison, but casting to 'int' just for silencing a
> > waring seems nuts.
> 
> i suspect the reasoning was one of readability --- keeping the [short]
> constants legible at the cost of making the expression slightly
> longer.
> 
> > This makes me wonder if breaking _those_ users could be a good thing...
> 
> like Paul Eggert said somewhere else today --- only if we're finding
> real bugs. and so far we're not.
> 
> it's like the warn_unused_result argument. a purist would argue that
> every function should have that annotation, because you should always
> check for errors, and if you're not already doing so, your code is
> already broken. whereas a pragmatist would argue that most people are
> just going to add the "shut up, compiler" cast (or disable the warning
> entirely) if their already-working code suddenly starts spamming
> warnings next time they build it.
> 
> while my bar for that might not be as high as my bar for ABI breakage,
> my source compatibility bar is still pretty high. it would be almost
> unethical of me to make app developers do random busywork. i have to
> be pretty confident (as with, say, "you just passed an fd > 1024 to an
> fd_set function/macro and thus corrupted memory") that their code is
> _definitely_ wrong. (and even there, that's going to have to be a
> runtime check!)

Yeah, I can agree with that.  I'm that kind of pedantic purist for my
own code, and it's painful that historic accidents like this one don't
allow me to be so in my own code.  But I agree that fixing the entire
world when their code is braindamaged but works is asking too much.

I'll just disable that pedantic warning when I use termbits.  :)

Btw, thanks for fixing that brain-damaged cast.  ;-)

Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: termios constants should be unsigned

[Zack Weinberg]

On Wed, Jun 12, 2024, at 10:55 AM, Paul Eggert wrote:
> A lot of this stuff is pedanticism that dates back to the bad old days
> when the C standard allowed ones' complement and signed magnitude
> representations of signed integers. Although it can be amusing to
> worry about that possibility (I know I've done it) it's never been a
> practical worry, and even the motivation of pedanticism is going away
> now that C23 requires two's complement.

Unless C23 eliminated *all* the cases where an operation on unsigned
integers is well-defined but the same operation on signed integers is
undefined, and last I checked it had not, there is still a need for
caution around conversions that change signedness.

zw

Re: termios constants should be unsigned

[Paul Eggert]

On 6/13/24 05:32, Zack Weinberg wrote:
> there is still a need for
> caution around conversions that change signedness.

Yes, just as there is need for caution around any use of unsigned types. 
Unfortunately in my experience Clang's (and even GCC's) warnings about 
signedness conversion are more likely to cause harm than good, with this 
thread being an example of the harm.

Part of the issue here is that GCC and Clang often do a better job of 
warning when constants are signed, not unsigned. For example, suppose a 
program mistakenly packages termios flags along with three other bits 
into an 'unsigned long', with code like this:

   unsigned long
   tagged_pendin (unsigned tag)
   {
     return (PENDIN << 3) | tag;
   }

Since PENDIN is 0x20000000 Clang and GCC by default warn about the 
mistake, as the signed integer overflow has undefined behavior. But if 
PENDIN were changed to 0x20000000U the behavior would be well-defined, 
there would be no warning even with -Wall -Wextra -Wsign-conversion, and 
the code would silently behave as if PENDIN were zero, which is not 
intended.

This is another reason why appending "U" to PENDIN's value would have 
drawbacks as well as advantages.

Re: termios constants should be unsigned

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

On Thu, Jun 13, 2024 at 02:12:20PM GMT, Paul Eggert wrote:
> Part of the issue here is that GCC and Clang often do a better job of
> warning when constants are signed, not unsigned. For example, suppose a
> program mistakenly packages termios flags along with three other bits into
> an 'unsigned long', with code like this:
> 
>   unsigned long
>   tagged_pendin (unsigned tag)
>   {
>     return (PENDIN << 3) | tag;
>   }
> 
> Since PENDIN is 0x20000000 Clang and GCC by default warn about the mistake,
> as the signed integer overflow has undefined behavior. But if PENDIN were
> changed to 0x20000000U the behavior would be well-defined, there would be no
> warning even with -Wall -Wextra -Wsign-conversion, and the code would
> silently behave as if PENDIN were zero, which is not intended.
> 
> This is another reason why appending "U" to PENDIN's value would have
> drawbacks as well as advantages.

Hmmmm, very interesting point!  I'll have that in mind when doing
bitwise stuff with constants.

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]