From owner-svn-src-head@FreeBSD.ORG Sat Jul 19 07:36:46 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2EA03F20; Sat, 19 Jul 2014 07:36:46 +0000 (UTC) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id E70512124; Sat, 19 Jul 2014 07:36:45 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id 7C7BC3C567D; Sat, 19 Jul 2014 17:36:42 +1000 (EST) Date: Sat, 19 Jul 2014 17:36:42 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Pedro F. Giffuni" Subject: Re: svn commit: r268867 - head/lib/libc/net In-Reply-To: <201407190153.s6J1rqBn027367@svn.freebsd.org> Message-ID: <20140719171552.W874@besplex.bde.org> References: <201407190153.s6J1rqBn027367@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=B4eAjodM c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=Nik8Deec72wA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=Q-jsWnCd4DCEJM4vVo8A:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 19 Jul 2014 07:36:46 -0000 On Sat, 19 Jul 2014, Pedro F. Giffuni wrote: > Log: > Use unsigned optlen in getsourcefilter() > > Sizes can not be negative and the functions that use it > expect an unsigned value anyways. > > Obtained from: Apple (Libc-997.90.3) > MFC after: 1 week Most uses of unsigned types are bugs. This one is an exception, but the change is still wrong. The critical use of the type needs it to be socklen_t, not int or unsigned int. socklen_t happens to have type uint32_t (unsigned due to old bugs). It only accidentally has the same size as unsigned int. It has a different type to plain int so compilers should warn about the type mismatch with certain warning flags. > Modified: > head/lib/libc/net/sourcefilter.c > > Modified: head/lib/libc/net/sourcefilter.c > ============================================================================== > --- head/lib/libc/net/sourcefilter.c Sat Jul 19 01:15:01 2014 (r268866) > +++ head/lib/libc/net/sourcefilter.c Sat Jul 19 01:53:52 2014 (r268867) > @@ -337,7 +337,8 @@ getsourcefilter(int s, uint32_t interfac > { > struct __msfilterreq msfr; > sockunion_t *psu; > - int err, level, nsrcs, optlen, optname; > + int err, level, nsrcs, optname; > + unsigned int optlen; This has mounds of style bugs. 2 more now (unsorting, and not using u_int, except u_int is not just a style bug) Other uses of this variable: % optlen = sizeof(struct __msfilterreq); The rvalue has type size_t. Unsigned is actually correct for size_t. The lvalue has a smaller type in many cases and we need to know that the result fits. We know that it is small, so if fits just as well in an int as in an u_int. It is less clear that it fits in a socklen_t since that is supposed to be opaque. % memset(&msfr, 0, optlen); memset() takes a size_t for its 3rd arg, but any type that can represent the value works provided memset()'s prototype is in scope. % err = _getsockopt(s, level, optname, &msfr, &optlen); This use needs the correct type since the reference is indirect so the prototype can't adjust the type. The arg had type "int *" in 4.4BSD but has suffered from typedef poisoning so it is now "socklen_t *" 4.4BSD also doesn't have socklen_t. socklen_t is specified by POSIX as being an integer type with width at least 32 bits. It is not required to be unsigned, and there are portability problems from this. POSIX recommends that applications not store values larger than 2**31-1 in socklen_t. 2**31 would only work if it is unsigned. Bruce