Bug 7472 - dn_expand() and res_query() used unconditionally in .../source3/libads/dns.c
dn_expand() and res_query() used unconditionally in .../source3/libads/dns.c
Status: RESOLVED WORKSFORME
Product: Samba 3.6
Classification: Unclassified
Component: Build environment
3.6.7
IA64 Other
: P3 normal
: ---
Assigned To: Björn Jacke
Samba QA Contact
:
Depends on: 9339
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-28 02:33 UTC by Joachim Schmitz
Modified: 2012-10-31 01:40 UTC (History)
1 user (show)

See Also:


Attachments
Patch (1.92 KB, patch)
2010-05-30 15:17 UTC, Joachim Schmitz
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joachim Schmitz 2010-05-28 02:33:40 UTC
dn_expand() and res_query() used unconditionally in .../source3/libads/dns.c.

Not sure at all how to properly fix it, but the least should be to brackt them which #ifdef HAVE_xxx:

diff -u ./source3/libads/dns.c.orig ./source3/libads/dns.c
--- ./source3/libads/dns.c.orig	2010-05-17 06:51:23.000000000 -0500
+++ ./source3/libads/dns.c	2010-05-27 07:30:42.000000000 -0500
@@ -87,7 +87,9 @@
 
 	/* See RFC 1035 for details. If this fails, then return. */
 
+#ifdef HAVE_DN_EXPAND /* TODO!!! */
 	namelen = dn_expand( start, end, p, hostname, sizeof(hostname) );
+#endif
 	if ( namelen < 0 ) {
 		return False;
 	}
@@ -124,7 +126,9 @@
 	ZERO_STRUCTP( rr );
 	/* pull the name from the answer */
 
+#ifdef HAVE_DN_EXPAND /* TODO!!! */
 	namelen = dn_expand( start, end, p, hostname, sizeof(hostname) );
+#endif
 	if ( namelen < 0 ) {
 		return -1;
 	}
@@ -198,7 +202,9 @@
 
 	p += 6;
 
+#ifdef HAVE_DN_EXPAND /* TODO!!! */
 	namelen = dn_expand( start, end, p, dcname, sizeof(dcname) );
+#endif
 	if ( namelen < 0 ) {
 		DEBUG(1,("ads_dns_parse_rr_srv: Failed to uncompress name!\n"));
 		return False;
@@ -247,7 +253,9 @@
 
 	/* ame server hostname */
 
+#ifdef HAVE_DN_EXPAND /* TODO!!! */
 	namelen = dn_expand( start, end, p, nsname, sizeof(nsname) );
+#endif
 	if ( namelen < 0 ) {
 		DEBUG(1,("ads_dns_parse_rr_ns: Failed to uncompress name!\n"));
 		return False;
@@ -336,6 +344,7 @@
 			}
 		}
 
#ifdef HAVE_RES_QUERY /* TODO!!! */
 		if ((resp_len = res_query(name, C_IN, q_type, buffer, buf_len))
 				< 0 ) {
 			DEBUG(3,("ads_dns_lookup_srv: "
@@ -353,6 +362,7 @@
 			last_dns_check = time(NULL);
 			return last_dns_status;
 		}
+#endif
 
 		/* On AIX, Solaris, and possibly some older glibc systems (e.g. SLES8)
 		   truncated replies never give back a resp_len > buflen


configure does check for dn_expand() (and sets HAVE_DN_EXPAND accordingly), but not for res_query(). I guess a system that does have dn_expand() would always also have res_query(), so that last chunk might better check for HAVE_DN_EXPAND rather than HAVE_RES_QUERY?

Bye, Jojo
Comment 1 Joachim Schmitz 2010-05-30 15:01:24 UTC
I ceated a more sensible patch, one that doesn't leave namelen uninitialized and does return sensible values (False resp. -1) to the caller. Will upload tomorrow.
Comment 2 Joachim Schmitz 2010-05-30 15:17:50 UTC
Created attachment 5750 [details]
Patch
Comment 3 Joachim Schmitz 2010-07-06 04:17:06 UTC
res_query() and dn_expand() are apparently part of glibc, but also part of bind.
So systems where glibc is not available might use bind's lib (-lbind) instead?
In any case Configure would need to check in addition to looking for it in -lresolv and set this up, wouldn't it?
Comment 4 Björn Jacke 2010-07-06 04:56:44 UTC
hm, I don't find them in my bind 9.7 header files. But in any case, yes, if we want to use those functions from another library we need to configure check for it. maybe putting the check and the "magic" to use libbind or whatever library into libreplace would be a good idea, too.
Comment 5 Joachim Schmitz 2010-07-06 04:58:43 UTC
I found them being defined in ...bind-9.2.2/lib/bind/resolv/res_{data,comp}.c
Comment 6 Joachim Schmitz 2011-03-01 06:08:18 UTC
A new version of a lib in NonStop now has dn_expand and res_query, however it is in a lib that is found via -linet rather than -lresolver.
This now allows for 2 alternatives to fix the issue

1. change configure.in to add
AC_CHECK_LIB(inet, dn_expand)
nearby it's check for
AC_CHECK_LIB(resolv, dn_expand)

2. change the wrapper cc we use on this platform to "translate" -lresolv to -linet

Which would you prefer?

Still a fix  is needed for those systems that don't have it at all.

Bye, Jojo
Comment 7 Björn Jacke 2012-09-15 23:37:34 UTC
the check for dn_expand in libinet is in master now, sorry for the dalay. which systems actually don't have dn_expand. I wonder if we really need to care about that actually now that NonStop added that also to libinet
Comment 8 Joachim Schmitz 2012-09-17 12:32:38 UTC
Please do, not all our customers are on a relasese that do have rn_expand and res_query.
Comment 9 Björn Jacke 2012-09-17 12:40:47 UTC
as we have no system to test this (all buildfarm systems of us do have dn_expand etc), can you propose a fix which makes use of an external library for that? Or do you mean the code should simply compile and leave the functionality broken (like attachment #5750 [details] does...)?
Comment 10 Joachim Schmitz 2012-09-17 12:42:25 UTC
(In reply to comment #9)
That 'leaving it broken' was good enough for me.
Comment 11 Joachim Schmitz 2012-09-17 12:46:21 UTC
(In reply to comment #9)
See also my comment #6, it is still not found by configure
Comment 12 Björn Jacke 2012-09-17 12:56:10 UTC
(In reply to comment #11)
> (In reply to comment #9)
> See also my comment #6, it is still not found by configure

you tested that with master and you configure output says:

checking for dn_expand in -linet... no

?


(In reply to comment #10)
> That 'leaving it broken' was good enough for me.

I'm not sure if that is what we want because we rely on the functionality of those functions and don't want to generate a binary which is half working. Any opinions on that from others here?
Comment 13 Joachim Schmitz 2012-09-17 13:52:55 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #9)
> > See also my comment #6, it is still not found by configure
> you tested that with master and you configure output says:
> checking for dn_expand in -linet... no

No I haven't tested this, but looking at master's configure I see no test for this, just a test for dn_expand in -lresolve. That would fail here.
-linet gets pulled in elsewhere (when checking for connect).

> (In reply to comment #10)
> > That 'leaving it broken' was good enough for me.
> I'm not sure if that is what we want because we rely on the functionality of
> those functions and don't want to generate a binary which is half working. Any
> opinions on that from others here?

I don't really like it much either, but apparently have no other choice. And it seems that code got never execises, at least I haven't seen failure die to it?

And I just found that we still run with this being disabled...
Comment 14 Joachim Schmitz 2012-09-18 15:02:00 UTC
samba master does checl for dn_expand in -linet, but doesn#t do anything is dn_expand isn't found at all.

As we have it since a while now and as that relaies is a prerequisit for running sambe for other reasosn already, I gues I can rest my case here and close the issue