From owner-freebsd-current@FreeBSD.ORG Wed Feb 20 19:42:37 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 3637CACC for ; Wed, 20 Feb 2013 19:42:37 +0000 (UTC) (envelope-from pluknet@gmail.com) Received: from mail-wg0-f46.google.com (mail-wg0-f46.google.com [74.125.82.46]) by mx1.freebsd.org (Postfix) with ESMTP id C9C9CF32 for ; Wed, 20 Feb 2013 19:42:36 +0000 (UTC) Received: by mail-wg0-f46.google.com with SMTP id fg15so6606406wgb.1 for ; Wed, 20 Feb 2013 11:42:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=n+pSHqR1EwQdqS3zbcHMNeTMGgZ9ueee210I/s1zTQk=; b=AMurDkfWL1ZOS2ZVwLG/LHtfEJpMGX+SbBGVX4bL9HinQDloZTy2TYAsfd+mbwGIwl Nv2IPL6U6sma8oHP/F2uAuBs9TE/kSljkEgnPVfPbyZO45QSewkLabheTt8bJIEs2s/H KjqHRS2c6lk4FtYbRuqeOZLVO1/5syO3l1LSLKUTXX3UyEuJp3RizWLdi+X0uBdc6rVA IcQjEQl+uAyjGaPIudSYOXWacPgZrtPv5eUF+kyHK2IglV4C3YE+/7TSDUvzpo32bPBD x67/jGx4rmzVr9E0hz2cXASso8AUAukMH96QQI3XGbKgr/eEPvKUUU7XlpQtnWuccU6j USQg== MIME-Version: 1.0 X-Received: by 10.194.235.6 with SMTP id ui6mr36519559wjc.12.1361389355616; Wed, 20 Feb 2013 11:42:35 -0800 (PST) Received: by 10.194.86.167 with HTTP; Wed, 20 Feb 2013 11:42:35 -0800 (PST) In-Reply-To: <5125191C.6010901@delphij.net> References: <5125191C.6010901@delphij.net> Date: Wed, 20 Feb 2013 22:42:35 +0300 Message-ID: Subject: Re: [patch] remove negative socklen_t checks From: Sergey Kandaurov To: d@delphij.net Content-Type: text/plain; charset=ISO-8859-1 Cc: FreeBSD Current X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2013 19:42:37 -0000 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