Subject: Re: patch for (possible) minor readability improvement

Re: patch for (possible) minor readability improvement

From: Erik Kline <ek_at_google.com>
Date: Thu, 29 Nov 2007 16:00:06 -0800

Oops! My bad. In the rework I dropped a ntohl(), which increased
unittesting finally showed up. Revised patch attached, and the fixed
section included below. Sorry for the incomplete/faulty testing
earlier.

-Erik

+ if (family == AF_INET)
+ {
+ in_addr_t s_addr = ntohl(addr->addr4.s_addr);
+ int a1 = (int)((s_addr >> 24) & 0xff);
+ int a2 = (int)((s_addr >> 16) & 0xff);
+ int a3 = (int)((s_addr >> 8) & 0xff);
+ int a4 = (int)(s_addr & 0xff);
+ snprintf(name, len, "%d.%d.%d.%d.in-addr.arpa", a4, a3, a2, a1);
+ }

On 29/11/2007, Erik Kline <ek_at_google.com> wrote:
> Basically this just moves all the address-family-specific PTR name
> construction out of ares_gethostbyaddr.c:next_lookup() into a separate
> function, ptr_rr_name(). No new feature or bug fix; unittested.
>
> Thanks,
> -Erik
>
> * inline patch:
>
> diff -u ares/ares_gethostbyaddr.c ares.test/ares_gethostbyaddr.c
> --- ares/ares_gethostbyaddr.c
> +++ ares.test/ares_gethostbyaddr.c
> @@ -58,6 +58,8 @@
> static void end_aquery(struct addr_query *aquery, int status,
> struct hostent *host);
> static int file_lookup(union ares_addr *addr, int family, struct
> hostent **host);
> +static void ptr_rr_name(char *name, size_t len, int family,
> + union ares_addr *addr);
>
> void ares_gethostbyaddr(ares_channel channel, const void *addr, int addrlen,
> int family, ares_host_callback callback, void *arg)
> @@ -101,44 +103,18 @@
> {
> const char *p;
> char name[128];
> - int a1, a2, a3, a4, status;
> + int status;
> struct hostent *host;
> - unsigned long addr;
>
> for (p = aquery->remaining_lookups; *p; p++)
> {
> switch (*p)
> {
> case 'b':
> - if (aquery->family == AF_INET)
> - {
> - addr = ntohl(aquery->addr.addr4.s_addr);
> - a1 = (int)((addr >> 24) & 0xff);
> - a2 = (int)((addr >> 16) & 0xff);
> - a3 = (int)((addr >> 8) & 0xff);
> - a4 = (int)(addr & 0xff);
> - sprintf(name, "%d.%d.%d.%d.in-addr.arpa", a4, a3, a2, a1);
> - aquery->remaining_lookups = p + 1;
> - ares_query(aquery->channel, name, C_IN, T_PTR, addr_callback,
> - aquery);
> - }
> - else
> - {
> - unsigned char *bytes;
> - bytes = (unsigned char *)&aquery->addr.addr6.s6_addr;
> - sprintf(name,
> "%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.ip6.arpa",
> - bytes[15]&0xf, bytes[15] >> 4, bytes[14]&0xf,
> bytes[14] >> 4,
> - bytes[13]&0xf, bytes[13] >> 4, bytes[12]&0xf,
> bytes[12] >> 4,
> - bytes[11]&0xf, bytes[11] >> 4, bytes[10]&0xf,
> bytes[10] >> 4,
> - bytes[9]&0xf, bytes[9] >> 4, bytes[8]&0xf, bytes[8] >> 4,
> - bytes[7]&0xf, bytes[7] >> 4, bytes[6]&0xf, bytes[6] >> 4,
> - bytes[5]&0xf, bytes[5] >> 4, bytes[4]&0xf, bytes[4] >> 4,
> - bytes[3]&0xf, bytes[3] >> 4, bytes[2]&0xf, bytes[2] >> 4,
> - bytes[1]&0xf, bytes[1] >> 4, bytes[0]&0xf,
> bytes[0] >> 4);
> - aquery->remaining_lookups = p + 1;
> - ares_query(aquery->channel, name, C_IN, T_PTR, addr_callback,
> - aquery);
> - }
> + ptr_rr_name(name, sizeof(name), aquery->family, &aquery->addr);
> + aquery->remaining_lookups = p + 1;
> + ares_query(aquery->channel, name, C_IN, T_PTR, addr_callback,
> + aquery);
> return;
> case 'f':
> status = file_lookup(&aquery->addr, aquery->family, &host);
> @@ -264,3 +240,32 @@
> *host = NULL;
> return status;
> }
> +
> +static void ptr_rr_name(char *name, size_t len, int family,
> + union ares_addr *addr) {
> + if (family == AF_INET)
> + {
> + int a1 = (int)((addr->addr4.s_addr >> 24) & 0xff);
> + int a2 = (int)((addr->addr4.s_addr >> 16) & 0xff);
> + int a3 = (int)((addr->addr4.s_addr >> 8) & 0xff);
> + int a4 = (int)(addr->addr4.s_addr & 0xff);
> + snprintf(name, len, "%d.%d.%d.%d.in-addr.arpa", a4, a3, a2, a1);
> + }
> + else
> + {
> + unsigned char *bytes = (unsigned char *)&addr->addr6.s6_addr;
> + snprintf(name, len,
> + "%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x."
> + "%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.%x.ip6.arpa",
> + bytes[15]&0xf, bytes[15] >> 4, bytes[14]&0xf, bytes[14] >> 4,
> + bytes[13]&0xf, bytes[13] >> 4, bytes[12]&0xf, bytes[12] >> 4,
> + bytes[11]&0xf, bytes[11] >> 4, bytes[10]&0xf, bytes[10] >> 4,
> + bytes[9]&0xf, bytes[9] >> 4, bytes[8]&0xf, bytes[8] >> 4,
> + bytes[7]&0xf, bytes[7] >> 4, bytes[6]&0xf, bytes[6] >> 4,
> + bytes[5]&0xf, bytes[5] >> 4, bytes[4]&0xf, bytes[4] >> 4,
> + bytes[3]&0xf, bytes[3] >> 4, bytes[2]&0xf, bytes[2] >> 4,
> + bytes[1]&0xf, bytes[1] >> 4, bytes[0]&0xf, bytes[0] >> 4);
> + }
> +}
>
>

Received on 2007-11-30