On Sat, 19 Jul 2014, Pedro Giffuni wrote:
Il giorno 19/lug/2014, alle ore 02:36, Bruce Evans <b...@optusnet.com.au> ha
scritto:
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.
Ah, yes, we have had this discussion before ? in relation with ext2fs ;).
Yes, it caused bugs in practice there.
The compiler doesn?t have a way to tell if socklen_t is of a certain type "by
accident?.
Yes, this is a fundamental problem with C typedefs -- they are too similar
to their underlying type.
Any size error would be detected more reliably than the sign error in
furture when int is expanded by socklen_t stays 32 bits. Except the
assumptions that int is 32 bits is even more established now than it
was on vaxes, so no one would expand int.
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.
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)
While aesthetically it may be better to keep the sorting, it doesn?t seem
correct to preserve it at the expense of setting the incorrect type. Would you
really want me to set optname in another line?
You already had to change the sorting to split the declaration. When
changing, it is better to put things in their correct place directly.
u_int is sorted before int because it is a more complex type with a
higher rank. socklen_t is sorted before int because it is a more
complex type with an unknown (opaque) relative rank.
% 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.
While locally the variable doesn?t need to be unsigned, conceptually it will
never be less than zero.
It is a common error to use unsigned types just because their value can
never be less than zero. Most systems of arithmetic (floating point,
bignums(?), ...) don't even have unsigned numbers.
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?s all just a waste of time I guess, so as I said, I won?t MFC the change.
It avoids warnings like:
z.c:7: warning: pointer targets in passing arg 1 of `strlen' differ in
signedness
This warning is under -pedantic for gcc, so it is rarely seen. Other
compilers may differ. With this warning and -Werror, you can't even
pass a "u_char *" pointer to strlen(). Strings consist of plain chars
so passing a "u_char *" to strlen() is a type mismatch. IIRC, the C
standard requires a diagnostic for this, but gcc is not a C compiler
even with -pedantic so it doesn't print the diagnostic without
-pedantic. This sign mismatch may be a serious error even for the
char type, but usually isn't.
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
