Date: Wed, 5 Feb 2014 14:22:25 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Eitan Adler <eadler@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r261502 - head/lib/libc/net Message-ID: <20140205135024.G1162@besplex.bde.org> In-Reply-To: <201402050200.s1520VvQ074916@svn.freebsd.org> References: <201402050200.s1520VvQ074916@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140205135024.G1162>