Re: [patch] remove negative socklen_t checks

2013-02-20 Thread Sergey Kandaurov
On 20 February 2013 22:42, Xin Li  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
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: [patch] remove negative socklen_t checks

2013-02-20 Thread Xin Li
-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).

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

Cheers,
- -- 
Xin LI https://www.delphij.net/
FreeBSD - The Power to Serve!   Live free or die
-BEGIN PGP SIGNATURE-

iQEcBAEBCgAGBQJRJRkcAAoJEG80Jeu8UPuzagkIAICE9uJzbLS8MvPPYLEMCop3
mamlq7AOJSpGfEyBzB0CZV2badJC91LEtUGADMN0CANPbvX6EpDsYoPygpXBuxei
etNelbp1+9jbqwV6yK+9FVCioRiMMLrPKkyh02+s1ZhWCf6kjz3Xl9MEYKUKYuV1
ay7xLcLnQMxOzf1oS7Sovm6NsIFnDC06WZ0PZDFdBtv7typtGblw3rrgWMsOnshL
x35C1UC8NgLnauMEOhX6Vr1zeRz+hqfw+YauCi/P+3chxfUgpe6XR551IN2h9xBU
mYTNEjLkRgX8u5sCHYGB16r/OZ3X59w1sJH/21ayrHuF0gNEmQbnMlBnA/krH94=
=iiGi
-END PGP SIGNATURE-
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


[patch] remove negative socklen_t checks

2013-02-20 Thread Sergey Kandaurov
Hi.

These checks are useless after the address length argument is converted
to socklen_t (up to SUSv2). Any objections?

Index: lib/libc/sys/accept.2
===
--- lib/libc/sys/accept.2   (revision 245745)
+++ lib/libc/sys/accept.2   (working copy)
@@ -28,7 +28,7 @@
 .\" @(#)accept.2   8.2 (Berkeley) 12/11/93
 .\" $FreeBSD$
 .\"
-.Dd December 11, 1993
+.Dd February 20, 2013
 .Dt ACCEPT 2
 .Os
 .Sh NAME
@@ -154,10 +154,6 @@ The descriptor references a file, not a socket.
 .It Bq Er EINVAL
 .Xr listen 2
 has not been called on the socket descriptor.
-.It Bq Er EINVAL
-The
-.Fa addrlen
-argument is negative.
 .It Bq Er EFAULT
 The
 .Fa addr
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);
}

AUDIT_ARG_FD(s);
@@ -1327,8 +1325,6 @@ kern_setsockopt(td, s, level, name, val, valseg, v

if (val == NULL && valsize != 0)
return (EFAULT);
-   if ((int)valsize < 0)
-   return (EINVAL);

sopt.sopt_dir = SOPT_SET;
sopt.sopt_level = level;
@@ -1406,8 +1402,6 @@ kern_getsockopt(td, s, level, name, val, valseg, v

if (val == NULL)
*valsize = 0;
-   if ((int)*valsize < 0)
-   return (EINVAL);

sopt.sopt_dir = SOPT_GET;
sopt.sopt_level = level;
@@ -1484,9 +1478,6 @@ kern_getsockname(struct thread *td, int fd, struct
socklen_t len;
int error;

-   if (*alen < 0)
-   return (EINVAL);
-
AUDIT_ARG_FD(fd);
error = getsock_cap(td->td_proc->p_fd, fd, CAP_GETSOCKNAME, &fp, NULL);
if (error)
@@ -1584,9 +1575,6 @@ kern_getpeername(struct thread *td, int fd, struct
socklen_t len;
int error;

-   if (*alen < 0)
-   return (EINVAL);
-
AUDIT_ARG_FD(fd);
error = getsock_cap(td->td_proc->p_fd, fd, CAP_GETPEERNAME, &fp, NULL);
if (error)

-- 
wbr,
pluknet
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"