[PATCH] Deduplicate resolv/nss_dns/dns-host.c

[PATCH] Deduplicate resolv/nss_dns/dns-host.c

[Ondřej Bílka]

Hi, when reviewing Andreas patch I noticed duplicated code. One way to
improve that is jump to exit logic. A second possibility would be put
 gaih_getanswer into a else block. What do you prefer?

	* resolv/nss_dns/dns-host.c (_nss_dns_gethostbyname4_r): Remove
	duplicate code.

diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
index 541c25a..4bf331e 100644
--- a/resolv/nss_dns/dns-host.c
+++ b/resolv/nss_dns/dns-host.c
@@ -342,16 +342,14 @@ _nss_dns_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat,
       else
 	__set_errno (olderr);
 
-      if (host_buffer.buf != orig_host_buffer)
-	free (host_buffer.buf);
-
-      return status;
+      goto free_and_return;
     }
 
   status = gaih_getanswer(host_buffer.buf, n, (const querybuf *) ans2p,
 			  resplen2, name, pat, buffer, buflen,
 			  errnop, herrnop, ttlp);
 
+ free_and_return:
   if (host_buffer.buf != orig_host_buffer)
     free (host_buffer.buf);
 

Re: [PATCH] Deduplicate resolv/nss_dns/dns-host.c

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]

On Thursday, February 13, 2014 13:38:12 Ondřej Bílka wrote:
> Hi, when reviewing Andreas patch I noticed duplicated code. One way to
> improve that is jump to exit logic. A second possibility would be put
>  gaih_getanswer into a else block. What do you prefer?

i don't mind either, but i think generally the preference is to avoid goto 
when easily possible.  so in this case, use the else.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v1.1] Deduplicate resolv/nss_dns/dns-host.c

[Ondřej Bílka]

On Fri, Feb 14, 2014 at 12:25:51PM -0500, Mike Frysinger wrote:
> On Thursday, February 13, 2014 13:38:12 Ondřej Bílka wrote:
> > Hi, when reviewing Andreas patch I noticed duplicated code. One way to
> > improve that is jump to exit logic. A second possibility would be put
> >  gaih_getanswer into a else block. What do you prefer?
> 
> i don't mind either, but i think generally the preference is to avoid goto 
> when easily possible.  so in this case, use the else.
> -mike

OK, patch for this is following. It should cover one extra possibility when
__libc_res_nsearch fails and allocates separate buffer.

	* resolv/nss_dns/dns-host.c (_nss_dns_gethostbyname4_r): Remove
	duplicate code

diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
index 365de70..47f998c 100644
--- a/resolv/nss_dns/dns-host.c
+++ b/resolv/nss_dns/dns-host.c
@@ -315,7 +315,13 @@ _nss_dns_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat,
   int n = __libc_res_nsearch (&_res, name, C_IN, T_UNSPEC,
 			      host_buffer.buf->buf, anslen, &host_buffer.ptr,
 			      &ans2p, &nans2p, &resplen2);
-  if (n < 0)
+  if (n >= 0)
+    {
+      status = gaih_getanswer (host_buffer.buf, n, (const querybuf *) ans2p,
+			       resplen2, name, pat, buffer, buflen,
+			       errnop, herrnop, ttlp);
+    }
+  else
     {
       switch (errno)
 	{
@@ -342,17 +348,8 @@ _nss_dns_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat,
 	*errnop = EAGAIN;
       else
 	__set_errno (olderr);
-
-      if (host_buffer.buf != orig_host_buffer)
-	free (host_buffer.buf);
-
-      return status;
     }
 
-  status = gaih_getanswer(host_buffer.buf, n, (const querybuf *) ans2p,
-			  resplen2, name, pat, buffer, buflen,
-			  errnop, herrnop, ttlp);
-
   /* Check whether ans2p was separately allocated.  */
   if (host_buffer.buf != orig_host_buffer)
     anslen = MAXPACKET;

Re: [PATCH v1.1] Deduplicate resolv/nss_dns/dns-host.c

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

On Saturday, February 15, 2014 12:39:19 Ondřej Bílka wrote:
> On Fri, Feb 14, 2014 at 12:25:51PM -0500, Mike Frysinger wrote:
> > On Thursday, February 13, 2014 13:38:12 Ondřej Bílka wrote:
> > > Hi, when reviewing Andreas patch I noticed duplicated code. One way to
> > > improve that is jump to exit logic. A second possibility would be put
> > > 
> > >  gaih_getanswer into a else block. What do you prefer?
> > 
> > i don't mind either, but i think generally the preference is to avoid goto
> > when easily possible.  so in this case, use the else.
> > -mike
> 
> OK, patch for this is following. It should cover one extra possibility when
> __libc_res_nsearch fails and allocates separate buffer.

OK
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v1.1] Deduplicate resolv/nss_dns/dns-host.c

[Allan McRae]

On 15/02/14 21:39, Ondřej Bílka wrote:
> On Fri, Feb 14, 2014 at 12:25:51PM -0500, Mike Frysinger wrote:
>> On Thursday, February 13, 2014 13:38:12 Ondřej Bílka wrote:
>>> Hi, when reviewing Andreas patch I noticed duplicated code. One way to
>>> improve that is jump to exit logic. A second possibility would be put
>>>  gaih_getanswer into a else block. What do you prefer?
>>
>> i don't mind either, but i think generally the preference is to avoid goto 
>> when easily possible.  so in this case, use the else.
>> -mike
> 
> OK, patch for this is following. It should cover one extra possibility when
> __libc_res_nsearch fails and allocates separate buffer.
> 
> 	* resolv/nss_dns/dns-host.c (_nss_dns_gethostbyname4_r): Remove
> 	duplicate code
> 

Ondřej: This was committed without a ChangeLog entry.  Can you insert
one in the appropriate place.

Thanks,
Allan

Re: [PATCH v1.1] Deduplicate resolv/nss_dns/dns-host.c

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

On Fri 04 Jul 2014 10:41:25 Allan McRae wrote:
> On 15/02/14 21:39, Ondřej Bílka wrote:
> > On Fri, Feb 14, 2014 at 12:25:51PM -0500, Mike Frysinger wrote:
> >> On Thursday, February 13, 2014 13:38:12 Ondřej Bílka wrote:
> >>> Hi, when reviewing Andreas patch I noticed duplicated code. One way to
> >>> improve that is jump to exit logic. A second possibility would be put
> >>> 
> >>>  gaih_getanswer into a else block. What do you prefer?
> >> 
> >> i don't mind either, but i think generally the preference is to avoid
> >> goto
> >> when easily possible.  so in this case, use the else.
> >> -mike
> > 
> > OK, patch for this is following. It should cover one extra possibility
> > when
> > __libc_res_nsearch fails and allocates separate buffer.
> > 
> > 	* resolv/nss_dns/dns-host.c (_nss_dns_gethostbyname4_r): Remove
> > 	duplicate code
> 
> Ondřej: This was committed without a ChangeLog entry.  Can you insert
> one in the appropriate place.

looks like Allan did this for you ~3 weeks later.  i wonder why he had to.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]