Re: Are clang unsigned comparison warnings in kern/kern_* ok?

On Wed Sep  5 12, Eir Nym wrote:
> I've got following warnings [no errors had been generated while
> -Werror is in command line] and want to know if they are ok. There are
> much more same warnings in modules, but I worry about kernel :
> Kernel config: http://eroese.org/_/_/pub/bsd/240070/GENERIC_PF.amd64
> src.conf: http://eroese.org/_/_/pub/bsd/240070/src.conf
> make.conf: /dev/null
> kernel build logs: http://eroese.org/_/_/pub/bsd/240070/kernel.amd64.GENERIC_PF

yes these are OK. clang complains about variables that were defined as
unsigned, but are being compared against < 0. since those variables cannot be
negative, comparing them against < 0 makes no sence (hence
-Wtautological-compare as name to define this group of warnings).

in theory calng is right, however in practice there might be good reasons to
do so:

1) as a general safety belt to make programmers feel comfortable
2) the check might be there, because in the future it is being planed to switch
   a certain variable from unsigned to signed
3) a variable type might be unsigned on one platform per default (no signed or
   unsigned keyword used), but might be signed on another. doing the < 0 check
   prevents programmers from having to deal with both cases.

you can disable these warnings for clang by setting -Wno-tautological-compare.
no idea however what var to put it in. CWARNFLAGS+= in src.conf maybe.

cheers.
alex

> 
> cc -c -O2 -pipe -fno-strict-aliasing  -std=c99  -Wall
> -Wredundant-decls -Wnested-externs -Wstrict-prototypes
> -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  -Wundef
> -Wno-pointer-sign -fformat-extensions  -Wmissing-include-dirs
> -fdiagnostics-show-option  -Wno-error-tautological-compare
> -Wno-error-empty-body  -Wno-error-parentheses-equality -nostdinc  -I.
> -I/usr/head/src/sys -I/usr/head/src/sys/contrib/altq -D_KERNEL
> -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h
> -fno-omit-frame-pointer -mno-aes -mno-avx -mcmodel=kernel
> -mno-red-zone -mno-mmx -mno-sse -msoft-float
> -fno-asynchronous-unwind-tables -ffreestanding -fstack-protector
> -Werror  /usr/head/src/sys/kern/kern_cpuset.c
> /usr/head/src/sys/kern/kern_cpuset.c:654:16: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>         for (i = 0; i < (_NCPUWORDS - 1); i++) {
>                     ~ ^ ~~~~~~~~~~~~~~~~
> 1 warning generated.
> 
> /usr/head/src/sys/kern/kern_poll.c:173:10: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>         if (val < 0 || val > 99)
>             ~~~ ^ ~
> 1 warning generated.
> 
> cc -c -O2 -pipe -fno-strict-aliasing  -std=c99  -Wall
> -Wredundant-decls -Wnested-externs -Wstrict-prototypes
> -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  -Wundef
> -Wno-pointer-sign -fformat-extensions  -Wmissing-include-dirs
> -fdiagnostics-show-option  -Wno-error-tautological-compare
> -Wno-error-empty-body  -Wno-error-parentheses-equality -nostdinc  -I.
> -I/usr/head/src/sys -I/usr/head/src/sys/contrib/altq -D_KERNEL
> -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h
> -fno-omit-frame-pointer -mno-aes -mno-avx -mcmodel=kernel
> -mno-red-zone -mno-mmx -mno-sse -msoft-float
> -fno-asynchronous-unwind-tables -ffreestanding -fstack-protector
> -Werror  /usr/head/src/sys/kern/kern_umtx.c
> /usr/head/src/sys/kern/kern_umtx.c:3312:19: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>                 if (ts32.tv_sec < 0 ||
>                     ~~~~~~~~~~~ ^ ~
> /usr/head/src/sys/kern/kern_umtx.c:3314:20: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>                     ts32.tv_nsec < 0)
>                     ~~~~~~~~~~~~ ^ ~
> /usr/head/src/sys/kern/kern_umtx.c:3338:25: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>         if (t32.timeout.tv_sec < 0 ||
>             ~~~~~~~~~~~~~~~~~~ ^ ~
> /usr/head/src/sys/kern/kern_umtx.c:3339:63: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>             t32.timeout.tv_nsec >= 1000000000 || t32.timeout.tv_nsec < 0)
>                                                  ~~~~~~~~~~~~~~~~~~~ ^ ~
> 4 warnings generated.
> 
> 
> cc -c -O2 -pipe -fno-strict-aliasing  -std=c99  -Wall
> -Wredundant-decls -Wnested-externs -Wstrict-prototypes
> -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  -Wundef
> -Wno-pointer-sign -fformat-extensions  -Wmissing-include-dirs
> -fdiagnostics-show-option  -Wno-error-tautological-compare
> -Wno-error-empty-body  -Wno-error-parentheses-equality -nostdinc  -I.
> -I/usr/head/src/sys -I/usr/head/src/sys/contrib/altq -D_KERNEL
> -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h
> -fno-omit-frame-pointer -mno-aes -mno-avx -mcmodel=kernel
> -mno-red-zone -mno-mmx -mno-sse -msoft-float
> -fno-asynchronous-unwind-tables -ffreestanding -fstack-protector
> -Werror  /usr/head/src/sys/kern/uipc_syscalls.c
> /usr/head/src/sys/kern/uipc_syscalls.c:356:16: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>                 if (*namelen < 0)
>                     ~~~~~~~~ ^ ~
> /usr/head/src/sys/kern/uipc_syscalls.c:1487:12: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>         if (*alen < 0)
>             ~~~~~ ^ ~
> /usr/head/src/sys/kern/uipc_syscalls.c:1587:12: warning: comparison of
> unsigned expression < 0 is always false [-Wtautological-compare]
>         if (*alen < 0)
>             ~~~~~ ^ ~
> 3 warnings generated.
> 
> -- Eir Nym