On Fri, Apr 26, 2019 at 01:55:52PM +0300, Vadim Penzin wrote: > Greetings, > > > 1. The manual of route(4) explains the structure of its messages thus: > > `Messages are formed by a header followed by a small number of > sockaddr structures (which are variable length), interpreted by > position, and delimited by the length entry in the sockaddr.' > > (That phrase is quite unfortunate in itself: `sa_len' is supposed to > /determine/ the length of a structure. To me, the word `delimited' > implies the presence of a memory object between two adjacent > structures, which certainly was not the intent of the author.)
Yes this is not quite right since the lenght is rounded to the next long word to ensure that structures are properly aligned on strict align architectures. > Armed with that knowledge, I wrote a parser the other day. I very > much enjoyed OpenBSD until my program fatally stumbled on a peculiar > structure that does not fit the above description: > > 05 00 00 00 ff 00 00 00 > > (The value of `sa_len' of that `sockaddr' is nothing that I am > familiar with, the address family is `AF_UNSPEC', and the most > disturbing: `sa_len' does not match the physical size of this > structure. These eight bytes were immediately followed by a perfectly > valid `sockaddr_in'.) This is a probably a netmask which are special in many senses from the old time of the radix routing tree. For AF_UNSPEC it defines how many bytes are used to store the netmask. It is wierd I agree. > It took me some time to figure out that, apparently, I am looking at > the combined effect of in_socktrim() (see src/sys/netinet/in.c) and > ROUNDUP() (see src/sys/net/rtsock.c) applied to the netmask of > 0xff000000 that was stored as sockaddr_in'. > > I believe that manuals should be more merciful. In the absence of > proper documentation, the part of my program that handles this case looks > like a mysterious ritual; it should not be that way. Please send suggestions to better documentation to this list. route(4) is not perfect for sure and could use some work. > 2. The definition of ROUNDUP() somewhat surprised me: it silently handles > zero. The macro is local to the compilation unit; it is used > this way: > > if (rtinfo == NULL || (sa = rtinfo->rti_info[i]) == NULL) > continue; > rtinfo->rti_addrs |= (1 << i); > dlen = ROUNDUP(sa->sa_len); > > and this way: > > if ((sa = rtinfo->rti_info[i]) == NULL) > continue; > rtinfo->rti_addrs |= (1 << i); > dlen = ROUNDUP(sa->sa_len); What code are you talking about? > (In the second case, the programmer does not check `rtinfo' for being > NULL. I only hope that there exists a good explanation to that.) > > 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. -- :wq Claudio
