On Sun, Jul 14, 2019 at 01:15:40PM +0300, Vadim Penzin wrote: > > > Since `sa_len' describes the size of a `sockaddr' (or one of its > > > derivatives) /including/ `sa_len' (maybe I am wrong, but this is my > > > interpretation of the comment `total length' that appears near the > > > definition of `struct sockaddr' in <sys/socket.h>), `sa_len' just > > > cannot be zero. > > > > Yes, it can not be zero. > > > The current definition of ROUNDUP() might hide a bug. In addition, on > > > some machines, it disturbs the pipeline of the CPU by introducing a > > > branch (for no real reason, as it seems, while I might be > > > nitpicking). At very least, it looks confusing. > > The following patch amends the issue: > > Index: route.c > =================================================================== > RCS file: /cvs/src/sys/net/route.c,v > retrieving revision 1.387 > diff -u -p -r1.387 route.c > --- route.c 24 Jun 2019 22:26:25 -0000 1.387 > +++ route.c 14 Jul 2019 10:05:04 -0000 > @@ -138,7 +138,7 @@ > #include <net/bfd.h> > #endif > > -#define ROUNDUP(a) (a>0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : > sizeof(long)) > +#define ROUNDUP(a) (1 + (((a) - 1) | (sizeof(long) - 1))) > > /* Give some jitter to hash, to avoid synchronization between routers. */ > static uint32_t rt_hashjitter; >
This only touches only one version of ROUNDUP() and looking at how ROUNDUP() is used in route.c I wonder if it is actually needed at all. Looking at the code in route.c and rtsock.c I think it is better to leave this like it is or fix both files making sure that the necessary checks are put in place to make sure that a sa_len of 0 can't get into the system. -- :wq Claudio
