Report information
The Basics
Id:
46520
Status:
resolved
Priority:
Low/Low
Queue:
bind9-public
People
Owner:
Nobody in particular
Requestors:
Mark Andrews <marka@isc.org>
Cc:
AdminCc:
BugTracker
Version Fixed:
9.12.0
Version Found:
(no value)
Versions Affected:
(no value)
Versions Planned:
9.12.0
Priority:
P1 High
Severity:
S1 High
CVSS Score:
(no value)
CVE ID:
(no value)
Component:
(no value)
Area:
bug
Dates
Created:Tue, 07 Nov 2017 19:03:48 -0500
Updated:Thu, 09 Nov 2017 20:19:25 -0500
Closed:Thu, 09 Nov 2017 20:19:22 -0500

This bug tracker is no longer active.

Please go to our Gitlab to submit issues (both feature requests and bug reports) for active projects maintained by Internet Systems Consortium (ISC).

Due to security and confidentiality requirements, full access is limited to the primary maintainers.

History
  Tue, 07 Nov 2017 19:03:50 -0500 Mark Andrews <marka@isc.org> - Ticket created
To: bind9-public@isc.org
Date: Tue, 07 Nov 2017 14:03:47 -1000
From: marka@isc.org
Subject: configure —use-inline-buffer=yes

We should add a configure option for setting ISC_BUFFER_USEINLINE and default that to yes rather that force the define in buffer.h so that we don't break existing code that just uses "#define ISC_BUFFER_USEINLINE" like dnsperf.
Tue, 07 Nov 2017 20:16:25 -0500 Mark Andrews <marka@isc.org> - Severity S1 High added
Tue, 07 Nov 2017 20:16:29 -0500 Mark Andrews <marka@isc.org> - Versions Planned 9.12.0 added
Tue, 07 Nov 2017 20:16:29 -0500 Mark Andrews <marka@isc.org> - Priority P1 High added
Tue, 07 Nov 2017 20:16:30 -0500 Mark Andrews <marka@isc.org> - Area feature changed to bug
  Tue, 07 Nov 2017 20:16:35 -0500 Mark Andrews <marka@isc.org> - Correspondence added

Ready for review. Changed the configure are to be be --enable-buffer-useinline. This is for 9.12.0.
Tue, 07 Nov 2017 20:16:49 -0500 Mark Andrews <marka@isc.org> - Status changed from 'open' to 'review'
  Wed, 08 Nov 2017 06:38:53 -0500 Michał Kępień <michal@isc.org> - Correspondence added

The fix proposed by rt46520 works correctly. However, I have mixed feelings about introducing workarounds to cater for external software doing things it apparently has no reason to do, like defining ISC_BUFFER_USEINLINE on its own. If we consider dnsperf, it seems the compiler was very clearly warning about this before commit 5c76f3664c broke compilation for good: $ make gcc -g -O2 -I/tmp/bind/include -D_REENTRANT -D_GNU_SOURCE -DPACKAGE_NAME=\"\" -DPACKAGE_TARNAME=\"\" -DPACKAGE_VERSION=\"\" -DPACKAGE_STRING=\"\" -DPACKAGE_BUGREPORT=\"\" -DHAVE_LIBNSL=1 -DHAVE_PTHREAD=1 -pthread -c dnsperf.c In file included from dnsperf.c:55:0: /tmp/bind/include/isc/buffer.h:110:0: warning: "ISC_BUFFER_USEINLINE" redefined #define ISC_BUFFER_USEINLINE 1 dnsperf.c:53:0: note: this is the location of the previous definition #define ISC_BUFFER_USEINLINE gcc -g -O2 -I/tmp/bind/include -D_REENTRANT -D_GNU_SOURCE -DPACKAGE_NAME=\"\" -DPACKAGE_TARNAME=\"\" -DPACKAGE_VERSION=\"\" -DPACKAGE_STRING=\"\" -DPACKAGE_BUGREPORT=\"\" -DHAVE_LIBNSL=1 -DHAVE_PTHREAD=1 -pthread -c datafile.c In file included from datafile.c:26:0: /tmp/bind/include/isc/buffer.h:110:0: warning: "ISC_BUFFER_USEINLINE" redefined #define ISC_BUFFER_USEINLINE 1 datafile.c:24:0: note: this is the location of the previous definition #define ISC_BUFFER_USEINLINE gcc -g -O2 -I/tmp/bind/include -D_REENTRANT -D_GNU_SOURCE -DPACKAGE_NAME=\"\" -DPACKAGE_TARNAME=\"\" -DPACKAGE_VERSION=\"\" -DPACKAGE_STRING=\"\" -DPACKAGE_BUGREPORT=\"\" -DHAVE_LIBNSL=1 -DHAVE_PTHREAD=1 -pthread -c dns.c In file included from dns.c:44:0: /tmp/bind/include/isc/buffer.h:110:0: warning: "ISC_BUFFER_USEINLINE" redefined #define ISC_BUFFER_USEINLINE 1 dns.c:41:0: note: this is the location of the previous definition #define ISC_BUFFER_USEINLINE Thus, I would rather report this upstream (possibly fixing the dnsperf copy in contrib/) than add a configure option to BIND as a workaround. Am I missing something?
Wed, 08 Nov 2017 06:38:54 -0500 Michał Kępień <michal@isc.org> - Given to Mark Andrews <marka@isc.org>
  Wed, 08 Nov 2017 07:07:14 -0500 Mark Andrews <marka@isc.org> - Correspondence added

On Wed Nov 08 01:38:53 2017, michal wrote: > The fix proposed by rt46520 works correctly. > > However, I have mixed feelings about introducing workarounds to cater > for external software doing things it apparently has no reason to do, > like defining ISC_BUFFER_USEINLINE on its own. If we consider > dnsperf, > it seems the compiler was very clearly warning about this before > commit > 5c76f3664c broke compilation for good: BIND 9.12 changed the documented interface for using the inline macros which dnsperf was perfectly entitled to use. That change is incompatible with the old method. See: git diff v9_11 v9_12_0b1 lib/isc/include/isc/buffer.h This reverts that change (back to just defining ISC_BUFFER_USEINLINE) and provides a alternate method to set the default.
  Wed, 08 Nov 2017 07:14:09 -0500 Michał Kępień <michal@isc.org> - Correspondence added

Ah, thanks. I missed the fact that the dnsperf warnings I quoted only started appearing after commit 03be5a6b4e, which is only present in master. In that case, sure, please go ahead and merge rt46520.
Wed, 08 Nov 2017 07:33:52 -0500 Mark Andrews <marka@isc.org> - Version Fixed 9.12.0 added
  Wed, 08 Nov 2017 07:33:52 -0500 Mark Andrews <marka@isc.org> - Correspondence added

4811. [bug] Revert api changes to use <isc/buffer.h> inline macros. Provide a alternative mechanism to turn on the use of inline macros when building BIND. [RT #46520]
Wed, 08 Nov 2017 07:33:53 -0500 Mark Andrews <marka@isc.org> - Status changed from 'review' to 'resolved'
Wed, 08 Nov 2017 07:33:54 -0500 Mark Andrews <marka@isc.org> - Untaken
  Wed, 08 Nov 2017 11:33:42 -0500 Evan Hunt <each@isc.org> - Correspondence added
From: "Evan Hunt" <each@isc.org>
To: "Michał Kępień via RT" <bind9-public@isc.org>
CC:
Date: Wed, 8 Nov 2017 16:33:41 +0000
Subject: Re: [ISC-Bugs #46520] configure —use-inline-buffer=yes

> Thus, I would rather report this upstream (possibly fixing the dnsperf > copy in contrib/) than add a configure option to BIND as a workaround. > Am I missing something? I think originally dnsperf was building fine, then we turned on ISC_BUFFER_USEINLINE in buffer.h and it started emitting a warning, then we turned it into a boolean value and it broke completely. Being performance-sensitive, it was reasonable for dnsperf to use this option, and it remains reasonable even if libisc isn't built with it. I'm fine with reporting it upstream and fixing it in contrib but I also think this is a good change to make.
Wed, 08 Nov 2017 11:33:43 -0500 The RT System itself - Status changed from 'resolved' to 'open'
Thu, 09 Nov 2017 20:19:25 -0500 Mark Andrews <marka@isc.org> - Status changed from 'open' to 'resolved'