Date: Sat, 19 Jul 2014 05:46:23 -0500 From: Pedro Giffuni <pfg@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> 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: <A66005FE-1A28-417A-8268-9690BC68EFD7@freebsd.org> In-Reply-To: <20140719171552.W874@besplex.bde.org> References: <201407190153.s6J1rqBn027367@svn.freebsd.org> <20140719171552.W874@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello;
Il giorno 19/lug/2014, alle ore 02:36, Bruce Evans =
<brde@optusnet.com.au> ha scritto:
> On Sat, 19 Jul 2014, Pedro F. Giffuni wrote:
>=20
>> Log:
>> Use unsigned optlen in getsourcefilter()
>>=20
>> Sizes can not be negative and the functions that use it
>> expect an unsigned value anyways.
>>=20
>> Obtained from: Apple (Libc-997.90.3)
>> MFC after: 1 week
>=20
> 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.
>=20
Ah, yes, we have had this discussion before =85 in relation with ext2fs =
;).
The compiler doesn=92t have a way to tell if socklen_t is of a certain =
type "by accident=94.
I will use socklen_t in this case instead of unsigned int, but I will =
not MFC the changes
as the original change has no functional (end-user) effect.
=20
>> Modified:
>> head/lib/libc/net/sourcefilter.c
>>=20
>> Modified: head/lib/libc/net/sourcefilter.c
>> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
>> --- 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;
>=20
> This has mounds of style bugs. 2 more now (unsorting, and not using
> u_int, except u_int is not just a style bug)
>=20
While aesthetically it may be better to keep the sorting, it doesn=92t =
seem correct to preserve it at the expense of setting the incorrect =
type. Would you really want me to set optname in another line?
> Other uses of this variable:
>=20
> % optlen =3D sizeof(struct __msfilterreq);
>=20
> 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.
>=20
> % memset(&msfr, 0, optlen);
>=20
> 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.
>=20
> % err =3D _getsockopt(s, level, optname, &msfr, &optlen);
>=20
> 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.
>=20
While locally the variable doesn=92t need to be unsigned, conceptually =
it will never be less than zero.
In this case, for the compiler, setting it to usigned int or to =
socklen_t makes no difference. In practice changing it from signed to =
unsigned has no effect either as the value will not be big enough to use =
the extra bit.
It=92s all just a waste of time I guess, so as I said, I won=92t MFC the =
change.
Pedro.
=20
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?A66005FE-1A28-417A-8268-9690BC68EFD7>