- Use getaddrinfo to replace gethostbyname.
- Use getnameinfo with NI_NAMEREQD flag to replace gethostbyaddr.
Details
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
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 58557 Build 55445: arc lint + arc unit
Event Timeline
| usr.sbin/arp/arp.c | ||
|---|---|---|
| 262–265 | Should I use memset to replace the bzero function? | |
This comment was removed by yanhaowang.
Comment Actions
Revise error code
We should use EAI_AGAIN instead of TRY_AGAIN error code while using getnameinfo.
Comment Actions
The change basically looks good to me, but I added some comments.
| usr.sbin/arp/arp.c | ||
|---|---|---|
| 263 | hints = (struct addrinfo){
.ai_family = AF_INET
};is enough for memset() + assignments. | |
| 611 | 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 | |
Comment Actions
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 | ||
|---|---|---|
| 265–270 | 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) {
...
} | |
| 266 | 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);
} | |
| 612 | Use sizeof(hostbuf) instead of NI_MAXHOST. | |
| usr.sbin/arp/arp_netlink.c | ||
| 189 | Same as L.613 in arp.c. | |
Comment Actions
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.