On Fri, Aug 05, 2022 at 10:00:26AM -0600, Theo de Raadt wrote: > For it to be an advantage, it must have actually helped at least once. > Did it find any pieces of code casting a struct mbuf * to a sockaddr *, > or similar?
I started to care about satosin with this bug: https://marc.info/?l=openbsd-cvs&m=136425691627698&w=2 https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/netinet6/in6_ifattach.c.diff?r1=1.59&r2=1.60&f=h At that time satosin6() was a #define which did not find the bug. So I changed the macros to inline functions (without copying from NetBSD). Then I made our kernel using these functions consistently. https://marc.info/?l=openbsd-cvs&m=136443074623462&w=2 Note that there was a lot of wrong usage of the #define version of the macros. They are really bad. Inline functions much stricter. > Or did it actually find zero, because programmers aren't that sloppy? They are: https://marc.info/?l=openbsd-cvs&m=160457319320094&w=2 https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/rtable.c.diff?r1=1.70&r2=1.71&f=h - if(((struct sockaddr_in *) - src->sa_data)->sin_addr.s_addr != INADDR_ANY) + if(satosin(src)->sin_addr.s_addr == INADDR_ANY) - if (!IN6_IS_ADDR_UNSPECIFIED(&((struct sockaddr_in6 *) - src->sa_data)->sin6_addr)) + if (IN6_IS_ADDR_UNSPECIFIED(&satosin6(src)->sin6_addr)) The wrong sa_data adds an offset of 2 to the address. New code is correct and easier to understand. > Should we mandate this development practice in all of userland No. > I am quite sick of pointless abstraction --- I think the tradeoff being made > here is that simple understanding of single lines of code is being damaged. We disagreed about the same issue 9 years ago. Currently our kernel has: - inline satosin functions where possible - no cast if types on both sides match - explicit casts only in strange situations, where you have to think I am happy with this situation. bluhm
