From owner-svn-src-all@FreeBSD.ORG Wed Feb 5 03:22:36 2014 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 56EE915D; Wed, 5 Feb 2014 03:22:36 +0000 (UTC) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id B9F161697; Wed, 5 Feb 2014 03:22:35 +0000 (UTC) Received: from c122-106-144-87.carlnfd1.nsw.optusnet.com.au (c122-106-144-87.carlnfd1.nsw.optusnet.com.au [122.106.144.87]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id B0C63782950; Wed, 5 Feb 2014 14:22:26 +1100 (EST) Date: Wed, 5 Feb 2014 14:22:25 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eitan Adler Subject: Re: svn commit: r261502 - head/lib/libc/net In-Reply-To: <201402050200.s1520VvQ074916@svn.freebsd.org> Message-ID: <20140205135024.G1162@besplex.bde.org> References: <201402050200.s1520VvQ074916@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=Wu4Sb7vv c=1 sm=1 tr=0 a=p/w0leo876FR0WNmYI1KeA==:117 a=PO7r1zJSAAAA:8 a=TLSZfMyrDvIA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=4jIg5_e19IMA:10 a=FCIpg6Y5KE6TmbiEcUQA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Feb 2014 03:22:36 -0000 On Wed, 5 Feb 2014, Eitan Adler wrote: > Log: > libc/net: fix a portability issue > > * POSIX does not require socklen_t to be unsigned > > Submitted by: bde Thanks. > Modified: head/lib/libc/net/ip6opt.c > ============================================================================== > --- head/lib/libc/net/ip6opt.c Wed Feb 5 00:26:11 2014 (r261501) > +++ head/lib/libc/net/ip6opt.c Wed Feb 5 02:00:31 2014 (r261502) > @@ -382,7 +382,7 @@ inet6_opt_init(void *extbuf, socklen_t e > struct ip6_ext *ext = (struct ip6_ext *)extbuf; > > if (ext) { > - if (extlen == 0 || (extlen % 8)) > + if (extlen <= 0 || (extlen % 8)) > return(-1); > ext->ip6e_len = (extlen >> 3) - 1; > } Hmm, this gives (restores) some other portability problems. Compilers may warn about "unsigned comparison with 0" on systems where socklen_t is unsigned. This is a general problem with typedefed types of unspecifed signedness. You don't want to ifdef the signed and unsigned cases to avoid this warning. While here it would be nice to fix some style bugs: - boolean comparison for the pointer 'ext' - this commit fixes the inconsistency not obfuscating the non-boolean comparison of 'extlen' similarly. ' == 0' looks strange when everywhere else omits both explicit comparisons with 0 and NULL. - excessive parentheses around 'extlen % 8' - boolean comparison for the non-boolean 'extlen % 8'. The excessive parentheses would have been non-excessive (needed) for a non-boolean comparison - no space before the return value. This file has many return statements and consistently misformats them all. Otherwise, its density of style bugs is only moderately high. Related to the original commit, the man page was confusing about the error checking for the extbuf != NULL case, and I don't remember the commit changing the man page. It says that "extlen is a positive multiple of 8". 0 is multiple of 8, and it is unclear if it is positive. 0 is in fact not allowed. Bruce