Skip site navigation (1)Skip section navigation (2)
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>