- Use getaddrinfo to replace gethostbyname.
- Use getnameinfo with NI_NAMEREQD flag to replace gethostbyaddr.
Details
print_entry function in "arp.c" will be call when WITHOUT_NETLINK is define and print_entry function in "arp_netlink.c" will be call in opsite. I run arp -a in both case on revise and not revised code. it output same like below.
// Original $ arp -a // WITHOUT_NETLINK is defined ? (192.168.42.2) at 00:50:56:ef:d2:41 on le0 expires in 1020 seconds [ethernet] ? (192.168.42.128) at 00:0c:29:02:ba:a1 on le0 permanent [ethernet] $ arp -a // WITHOUT_NETLINK is not defined ? (192.168.42.2) at 00:50:56:ef:d2:41 on le0 expires in 961 seconds [ethernet] ? (192.168.42.128) at 00:0c:29:02:ba:a1 on le0 permanent [ethernet] // Revised $ arp -a // WITHOUT_NETLINK is defined ? (192.168.42.2) at 00:50:56:ef:d2:41 on le0 expires in 1093 seconds [ethernet] ? (192.168.42.128) at 00:0c:29:02:ba:a1 on le0 permanent [ethernet] $ arp -a // WITHOUT_NETLINK is not defined ? (192.168.42.2) at 00:50:56:ef:d2:41 on le0 expires in 1175 seconds [ethernet] ? (192.168.42.128) at 00:0c:29:02:ba:a1 on le0 permanent [ethernet]
As for the getaddr function I use sudo arp -d kola (kola is a hostname of my private IP, for testing the getaddrinfo function work) and sudo arp -d -a to test and it work fine (successfully delete the entry).
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 58871 Build 55758: arc lint + arc unit
Event Timeline
usr.sbin/arp/arp.c | ||
---|---|---|
262–267 | Should I use memset to replace the bzero function? |
Revise error code
We should use EAI_AGAIN instead of TRY_AGAIN error code while using getnameinfo.
The change basically looks good to me, but I added some comments.
usr.sbin/arp/arp.c | ||
---|---|---|
265 | hints = (struct addrinfo){ .ai_family = AF_INET }; is enough for memset() + assignments. | |
613 | sizeof() should be replaced with addr->sin_len. I would recommend to change the type of addr to struct sockaddr, and define struct sockaddr_in *sin = (struct sockaddr_in *)addr at the top of this function to use it as sockaddr_in. This requires additional changes in search() and action_fn, though. | |
usr.sbin/arp/arp_netlink.c | ||
188 | The same as arp.c |
Revise according Hiroki advise
- Use sin_len member.
- Change function prototype from sockaddr_in to sockaddr. In this case we change action_fn, print_entry and nuke_entry function.
- Use struct initializer to replace memset and assignments.
usr.sbin/arp/arp.c | ||
---|---|---|
267–272 | Removing "hints" and use of compound literals will give a more compact idiom like this: error = getaddrinfo(host, NULL, &(struct addrinfo){ .ai_family = AF_INET }, &res); I would recommend to use "error" instead of "status" as the variable name because the conditional can be written like the following. I believe it is more intuitive: if (error) { ... } | |
268 | Check whether the return value is EAI_SYSTEM or not. If it is EAI_SYSTEM, the error code must be extracted from errno, not the return value. if (status != 0) { xo_warnx("%s: %s", host, (status == EAI_SYSTEM) ? strerror(errno) : gai_strerror(status)); return (NULL); } | |
614 | Use sizeof(hostbuf) instead of NI_MAXHOST. | |
usr.sbin/arp/arp_netlink.c | ||
189 | Same as L.613 in arp.c. |
Update according Hiroki advise
- Use "error" instead of "status" name to store the value of getnameinfo() function.
- Use compound literals instead of "hints" variable.
- Check EAI_SYSTEM while using gai_strerror() function.
- Use sizoef(hostbuf) instead of NI_MAXHOST.