Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Feb 2013 22:42:35 +0300
From:      Sergey Kandaurov <pluknet@gmail.com>
To:        d@delphij.net
Cc:        FreeBSD Current <freebsd-current@freebsd.org>
Subject:   Re: [patch] remove negative socklen_t checks
Message-ID:  <CAE-mSOJExSsOakcmY5jOP_-eVdhhv33h1WoK=62Rg_saH9LJjA@mail.gmail.com>
In-Reply-To: <5125191C.6010901@delphij.net>
References:  <CAE-mSOKJHqov7kHKpKFRw%2Bcq5W%2B6du88GVNa2xvfLaoeO%2BE%2BuA@mail.gmail.com> <5125191C.6010901@delphij.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On 20 February 2013 22:42, Xin Li <delphij@delphij.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> On 02/20/13 09:19, Sergey Kandaurov wrote:
>> Hi.
>>
>> These checks are useless after the address length argument is
>> converted to socklen_t (up to SUSv2). Any objections?
>
> No objection in general but there is a minor style issue, see below.
>
> [...]
>> Index: sys/kern/uipc_syscalls.c
>> ===================================================================
>>
>>
> - --- sys/kern/uipc_syscalls.c    (revision 246354)
>> +++ sys/kern/uipc_syscalls.c    (working copy) @@ -353,8 +353,6 @@
>> kern_accept(struct thread *td, int s, struct socka
>>
>> if (name) { *name = NULL; -               if (*namelen < 0) -
>> return (EINVAL); }
>
> The { } for if () is no longer needed now per style(9).

Indeed, thanks.

>
> By the way I wonder why there is no compiler warning for this -- or do
> we compile kernel without signedness warnings?  Just curious...

No, they are (at least with clang). That's how I came there.

cc -c -O2 -pipe -fno-strict-aliasing  -std=c99 -g -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/src/sys -I/usr/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/src/sys/kern/uipc_syscalls.c
/usr/src/sys/kern/uipc_syscalls.c:356:16: warning: comparison of
unsigned expression < 0 is always false [-Wtautological-compare]
                if (*namelen < 0)
                    ~~~~~~~~ ^ ~
/usr/src/sys/kern/uipc_syscalls.c:1487:12: warning: comparison of
unsigned expression < 0 is always false [-Wtautological-compare]
        if (*alen < 0)
            ~~~~~ ^ ~
/usr/src/sys/kern/uipc_syscalls.c:1587:12: warning: comparison of
unsigned expression < 0 is always false [-Wtautological-compare]
        if (*alen < 0)
            ~~~~~ ^ ~

Other two warnings are obviously not triggered because of explicit
cast to int (eek!).
Thanks for review.

-- 
wbr,
pluknet



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAE-mSOJExSsOakcmY5jOP_-eVdhhv33h1WoK=62Rg_saH9LJjA>