Date: Sat, 19 Jul 2014 17:36:42 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "Pedro F. Giffuni" <pfg@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r268867 - head/lib/libc/net Message-ID: <20140719171552.W874@besplex.bde.org> In-Reply-To: <201407190153.s6J1rqBn027367@svn.freebsd.org> References: <201407190153.s6J1rqBn027367@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140719171552.W874>