Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor compat/getnameinfo.cc #619

Closed
wants to merge 3 commits into from
Closed

Refactor compat/getnameinfo.cc #619

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17fc81f
Refactor compact/getnameinfo.cc
kinkie Apr 28, 2020
03566a6
more getnameinfo.cc refactor
kinkie Apr 28, 2020
ebf53a8
Merge branch 'master' into refactor-getnameinfo
kinkie Apr 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 28 additions & 80 deletions compat/getnameinfo.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,15 @@ static const struct afd {
int a_socklen;
int a_off;
int a_portoff;
} afdl [] = {
} afdl[] = {
#if INET6
{ PF_INET6, sizeof(struct in6_addr), sizeof(struct sockaddr_in6),
offsetof(struct sockaddr_in6, sin6_addr),
offsetof(struct sockaddr_in6, sin6_port)
},
{PF_INET6, sizeof(struct in6_addr), sizeof(struct sockaddr_in6),
offsetof(struct sockaddr_in6, sin6_addr),
offsetof(struct sockaddr_in6, sin6_port)},
#endif
{ PF_INET, sizeof(struct in_addr), sizeof(struct sockaddr_in),
offsetof(struct sockaddr_in, sin_addr),
offsetof(struct sockaddr_in, sin_port)
},
{PF_INET, sizeof(struct in_addr), sizeof(struct sockaddr_in),
offsetof(struct sockaddr_in, sin_addr),
offsetof(struct sockaddr_in, sin_port)},
{0, 0, 0, 0, 0},
};

Expand All @@ -162,14 +160,9 @@ xgetnameinfo(const struct sockaddr *sa, socklen_t salen, char *host, size_t host
uint32_t v4a;
char numserv[512];

if (sa == NULL)
if (!sa)
return EAI_FAIL;

#if HAVE_SA_LEN /*XXX*/
if (sa->sa_len != salen)
return EAI_FAIL;
#endif

family = sa->sa_family;
for (i = 0; afdl[i].a_af; i++)
if (afdl[i].a_af == family) {
Expand All @@ -186,7 +179,7 @@ xgetnameinfo(const struct sockaddr *sa, socklen_t salen, char *host, size_t host
memcpy(&port, (const char *)sa + afd->a_portoff, sizeof(port));
addr = (const char *)sa + afd->a_off;

if (serv == NULL || servlen == 0) {
if (!serv || servlen == 0) {
/*
* do nothing in this case.
* in case you are wondering if "&&" is more correct than
Expand Down Expand Up @@ -215,15 +208,15 @@ xgetnameinfo(const struct sockaddr *sa, socklen_t salen, char *host, size_t host
switch (sa->sa_family) {
case AF_INET:
v4a = (uint32_t)
ntohl(((const struct sockaddr_in *)sa)->sin_addr.s_addr);
ntohl(((const struct sockaddr_in *)sa)->sin_addr.s_addr);
if (IN_MULTICAST(v4a) || IN_EXPERIMENTAL(v4a))
flags |= NI_NUMERICHOST;
v4a >>= IN_CLASSA_NSHIFT;
if (v4a == 0)
flags |= NI_NUMERICHOST;
break;
#if INET6
case AF_INET6: {
case AF_INET6:
const struct sockaddr_in6 *sin6;
sin6 = (const struct sockaddr_in6 *)sa;
switch (sin6->sin6_addr.s6_addr[0]) {
Expand All @@ -242,11 +235,10 @@ xgetnameinfo(const struct sockaddr *sa, socklen_t salen, char *host, size_t host
flags |= NI_NUMERICHOST;
break;
}
}
break;
break;
#endif
}
if (host == NULL || hostlen == 0) {
if (!host || hostlen == 0) {
/*
* do nothing in this case.
* in case you are wondering if "&&" is more correct than
Expand All @@ -260,87 +252,45 @@ xgetnameinfo(const struct sockaddr *sa, socklen_t salen, char *host, size_t host

goto numeric;
} else {
#if USE_GETIPNODEBY
int h_error = 0;
hp = getipnodebyaddr(addr, afd->a_addrlen, afd->a_af, &h_error);
#else
hp = gethostbyaddr(addr, afd->a_addrlen, afd->a_af);
#if 0 // getnameinfo.c:161:9: error: variable 'h_error' set but not used
#if HAVE_H_ERRNO
h_error = h_errno;
#else
h_error = EINVAL;
#endif
#endif /* 0 */
#endif

if (hp) {
#if 0
if (flags & NI_NOFQDN) {
/*
* According to RFC3493 section 6.2, NI_NOFQDN
* means "node name portion of the FQDN shall
* be returned for local hosts." The following
* code tries to implement it by returning the
* first label (the part before the first
* period) of the FQDN. However, it is not
* clear if this always makes sense, since the
* given address may be outside of "local
* hosts." Due to the unclear description, we
* disable the code in this implementation.
*/
char *p;
p = strchr(hp->h_name, '.');
if (p)
*p = '\0';
}
#endif
if (strlen(hp->h_name) + 1 > hostlen) {
#if USE_GETIPNODEBY
freehostent(hp);
#endif
return EAI_OVERFLOW;
}
xstrncpy(host, hp->h_name, hostlen);
#if USE_GETIPNODEBY
freehostent(hp);
#endif
} else {
if (flags & NI_NAMEREQD)
return EAI_NONAME;

numeric:
numeric:
switch (afd->a_af) {
#if INET6
case AF_INET6: {
case AF_INET6:
int error;
Comment on lines 268 to 270
Copy link
Contributor

@rousskov rousskov Apr 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing these curly braces will lead to compile errors if this code is ever compiled: https://stackoverflow.com/questions/92396/why-cant-variables-be-declared-in-a-switch-statement

Please note that the #if INET6 guard is probably a bug. Fetchmail defines that macro and Squid does not:

configure.ac:    AC_DEFINE(INET6,1,Define to 1 if your system defines AF_INET6 and PF_INET6.)

This problem highlights why it is usually a bad idea to polish code without proper testing.


if ((error = ip6_parsenumeric(sa, addr, host,
hostlen,
flags)) != 0)
return(error);
flags))
!= 0)
return error;
break;
}
#endif
default:
if (inet_ntop(afd->a_af, addr, host,
hostlen) == NULL)
hostlen)
== NULL)
return EAI_SYSTEM;
break;
}
}
}
return(0);
return 0;
}

#if INET6
static int
ip6_parsenumeric(sa, addr, host, hostlen, flags)
const struct sockaddr *sa;
const char *addr;
char *host;
size_t hostlen;
int flags;
ip6_parsenumeric(const struct sockaddr *sa, const char *addr, char *host, size_t hostlen, int flags)
{
int numaddrlen;
char numaddr[512];
Expand All @@ -358,8 +308,8 @@ int flags;
int zonelen;

zonelen = ip6_sa2str(
(const struct sockaddr_in6 *)(const void *)sa,
zonebuf, sizeof(zonebuf), flags);
reinterpret_cast<const struct sockaddr_in6 *>(sa),
zonebuf, sizeof(zonebuf), flags);
if (zonelen < 0)
return EAI_OVERFLOW;
if (zonelen + 1 + numaddrlen + 1 > hostlen)
Expand All @@ -377,8 +327,8 @@ int flags;

/* ARGSUSED */
static int
ip6_sa2str(sa6, buf, bufsiz, flags)
const struct sockaddr_in6 *sa6;
ip6_sa2str(sa6, buf, bufsiz, flags)
const struct sockaddr_in6 *sa6;
char *buf;
size_t bufsiz;
int flags;
Expand All @@ -401,11 +351,10 @@ int flags;
#endif

/* if_indextoname() does not take buffer size. not a good api... */
if ((IN6_IS_ADDR_LINKLOCAL(a6) || IN6_IS_ADDR_MC_LINKLOCAL(a6) ||
IN6_IS_ADDR_MC_NODELOCAL(a6)) && bufsiz >= IF_NAMESIZE) {
if ((IN6_IS_ADDR_LINKLOCAL(a6) || IN6_IS_ADDR_MC_LINKLOCAL(a6) || IN6_IS_ADDR_MC_NODELOCAL(a6)) && bufsiz >= IF_NAMESIZE) {
char *p = if_indextoname(ifindex, buf);
if (p)
return (strlen(p));
return strlen(p);
}

/* last resort */
Expand All @@ -417,4 +366,3 @@ int flags;
}
#endif /* INET6 */
#endif /* HAVE_DECL_GETNAMEINFO */