arp.c: use getaddrinfo(3) instead of gethostbyname(3)

[hrs]

usr.sbin/arp/arp.c

274–275

I suggest to rename reply to sin in addition to the other changes.

275–292

Use memset() instead of bzero() for userland utilities.

276

hints.ai_family must be used here.

281

xo_errx() does not return.

295

An assertion for res->ai_addrlen == sizeof(struct sockaddr_in) might be worth being added before memcpy(). And I personally think that an warning should be displayed when getaddrinfo() returns multiple addresses.

[luthramihir708_gmail.com]

1. Updating D21589: arp.c: use getaddrinfo(3) instead of gethostbyname(3) #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

1)changed variable name 'reply' to 'sin'
2)use memset(3) instead of bzero(3)
3)bug fix: AF_INET flag supplied in hints.ai_family instead of hints.ai_flags
4)replaced xo_errx(3) with xo_warnx(3) when host wasn't resolved
5)add assrtion when getaddrinfo doesn't return an ipv4 address
6)add warning when getaddrinfo returns multiple addresses

[luthramihir708_gmail.com]

usr.sbin/arp/arp.c

276

Sorry that was by mistake. Thanks

295

If getaddrinfo(3) returns multiple addresses, should I loop to check if any of them is ipv4. If I am not wrong, arp is meant only for ipv4 right?
Thanks for the review :)

295

I was thinking if I am specifying family as AF_INET, it shouldn't return any sockaddr_in6. Shouldn't it be fine without check? Can ai_addr contain anything except sockaddr_in when family is AF_INET?

[luthramihir708_gmail.com]

1. Updating D21589: arp.c: use getaddrinfo(3) instead of gethostbyname(3) #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Removed unecessary warning

[hrs]

usr.sbin/arp/arp.c

285

I think "%s: %s" should be enough here as in the original version. gai_strerror() will put a description such as "Name does not resolve" when EAI_NONAME, for example.

295

What I suggested was use of assert(), not a conditional, for debugging purpose. This is not directly related to AF_INET6 or others which getaddrinfo(3) can return (and you seem to miss a possibility that getaddrinfo(3) can return other than AF_INET{,6}). The reason why adding assert(res->ai_addrlen == sizeof(sin)) is that ai_addrlen is not constant. A buffer length check in C language must be conservative---even if someone may mistakenly remove hints.ai_family = AF_INET in the future, the assert() eliminates possibility of buffer overrun.

296

Please remove the trailing whitespaces of this line. When multiple addresses are found, I think arp(8) should abort (i.e. this function returns NULL) because it is quite likely against the sysadmin's intention.

[luthramihir708_gmail.com]

Made corrections as suggested.

[luthramihir708_gmail.com]

usr.sbin/arp/arp.c

295

Got it. I will remember to consider such scenarios too. Thanks.

[hrs]

Looks good to me.

[asomers]

@hrs do you realize that the submitter is not a committer? Can you commit this change for him?

[hrs]

In D21589#481177, @asomers wrote:

@hrs do you realize that the submitter is not a committer? Can you commit this change for him?

Yes. I will commit this patch shortly.