Report information
The Basics
Id:
46201
Status:
resolved
Priority:
Low/Low
Queue:
bind9-public
People
BugTracker
Version Fixed:
9.12.0
Version Found:
(no value)
Versions Affected:
(no value)
Versions Planned:
(no value)
Priority:
P3 Low
Severity:
S3 Low
CVSS Score:
(no value)
CVE ID:
(no value)
Component:
BIND Common
Area:
feature
Dates
Created:Thu, 05 Oct 2017 08:01:11 -0400
Updated:Wed, 11 Oct 2017 03:32:27 -0400
Closed:Mon, 09 Oct 2017 05:46:17 -0400

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
  Thu, 05 Oct 2017 08:01:11 -0400 Michał Kępień <michal@isc.org> - Ticket created
Date: Thu, 05 Oct 2017 14:01:11 +0200
To: bind9-public@isc.org
Subject: Add isc_buffer_printf()
From: michal@isc.org

While RT #46130 ("Drop dependency on libxml2 and libjson") will likely be rejected, it introduces a useful new function, isc_buffer_printf(), which should be added to the source code nevertheless.
Thu, 05 Oct 2017 08:09:04 -0400 Michał Kępień <michal@isc.org> - Reference to #46130: Drop dependency on libxml2 and libjson added
  Thu, 05 Oct 2017 08:09:28 -0400 Michał Kępień <michal@isc.org> - Correspondence added

rt46201 contains Mukund's original implementation augmented by a minor bug fix, documentation and unit tests. Please review.
Thu, 05 Oct 2017 08:09:29 -0400 Michał Kępień <michal@isc.org> - Status changed from 'open' to 'review'
Thu, 05 Oct 2017 08:09:29 -0400 Michał Kępień <michal@isc.org> - Untaken
  Thu, 05 Oct 2017 15:36:28 -0400 Evan Hunt <Evan_Hunt@isc.org> - Correspondence added

A couple of suggestions: - there's no reason to require autoreallocation in buffers; if a buffer can't be reallocated and we're trying to reserve too much space, then isc_buffer_reserve() just returns ISC_R_NOSPACE. - we can work out the number of characters added to a buffer in other ways, but it's useful to float result codes up the call stack, so let's have isc_buffer_printf() return isc_result_t. I've pushed a commit that makes these changes - what do you think?
Thu, 05 Oct 2017 15:36:29 -0400 Evan Hunt <Evan_Hunt@isc.org> - Given to Michał Kępień <michal@isc.org>
  Thu, 05 Oct 2017 20:07:29 -0400 Evan Hunt <Evan_Hunt@isc.org> - Correspondence added

It occurs to me that isc_buffer_sprintf() might be a more intuitive name for what this does. Pushed. Also changed some existing snprintf() calls to use it. Seems to work fine.
  Fri, 06 Oct 2017 03:52:35 -0400 Mukund Sivaraman <muks@isc.org> - Correspondence added
From: "Mukund Sivaraman" <muks@isc.org>
Subject: Re: [ISC-Bugs #46201] Add isc_buffer_printf()
To: "Evan Hunt via RT" <bind9-public@isc.org>
Date: Fri, 6 Oct 2017 13:22:32 +0530

On Fri, Oct 06, 2017 at 12:07:30AM +0000, Evan Hunt via RT wrote: > It occurs to me that isc_buffer_sprintf() might be a more intuitive name > for what this does. Pushed. Print to string: (s)printf Print to file: (f)printf Print to buffer: (isc_buffer_)printf Mukund
  Fri, 06 Oct 2017 09:43:03 -0400 Michał Kępień <michal@isc.org> - Correspondence added

Evan, Good call on adding support for static buffers as well, this will make this function more versatile. I just pushed a few further tweaks (I explained the reasoning behind each change in commit log messages) and reverted the name change as Mukund makes a good point IMHO. As most of these latest modifications are basically bikeshedding, feel free to revert them to any extent you deem appropriate if you have strong feelings about any of them. Once you are happy with the end result, please let me know and I will merge it. Thanks!
Fri, 06 Oct 2017 09:43:04 -0400 Michał Kępień <michal@isc.org> - Given to Evan Hunt <Evan_Hunt@isc.org>
  Fri, 06 Oct 2017 13:34:22 -0400 Evan Hunt <each@isc.org> - Correspondence added
CC:
Subject: Re: [ISC-Bugs #46201] Add isc_buffer_printf()
From: "Evan Hunt" <each@isc.org>
Date: Fri, 6 Oct 2017 17:34:20 +0000
To: "Mukund Sivaraman via RT" <bind9-public@isc.org>

> Print to string: (s)printf > Print to file: (f)printf > Print to buffer: (isc_buffer_)printf True, and I did think of that, but my brain tends to look at what's after the last underscore as the main indicator of what a function does, so to me isc_buffer_printf() sounds like it should print the content of a buffer to stdout. However, since I seem to be the only one with that quirk, I'm fine with the original name.
  Fri, 06 Oct 2017 13:42:49 -0400 Evan Hunt <each@isc.org> - Correspondence added
To: "Michał Kępień via RT" <bind9-public@isc.org>
From: "Evan Hunt" <each@isc.org>
Date: Fri, 6 Oct 2017 17:42:48 +0000
Subject: Re: [ISC-Bugs #46201] Add isc_buffer_printf()
CC:

On Fri, Oct 06, 2017 at 01:43:03PM +0000, Michał Kępień via RT wrote: > Once you are happy with the end result, please let me know and I > will merge it. My only comment is that isc_httpd_*() functions were previously always returning ISC_R_SUCCESS and now they aren't. I do think that's an improvement, but my thought was to leave the behavior of those functions identical. However, since the result codes aren't being caught by the caller anyway, let's go ahead with it. Okay to merge.
Fri, 06 Oct 2017 18:05:22 -0400 Evan Hunt <Evan_Hunt@isc.org> - Given to Michał Kępień <michal@isc.org>
  Mon, 09 Oct 2017 05:46:16 -0400 Michał Kępień <michal@isc.org> - Correspondence added

4767. [func] Add a new function, isc_buffer_printf(), which can be used to append a formatted string to the used region of a buffer. [RT #46201] 9.12.0
Mon, 09 Oct 2017 05:46:17 -0400 Michał Kępień <michal@isc.org> - Status changed from 'review' to 'resolved'
Mon, 09 Oct 2017 05:46:17 -0400 Michał Kępień <michal@isc.org> - Version Fixed 9.12.0 added
Wed, 11 Oct 2017 03:32:27 -0400 Michał Kępień <michal@isc.org> - Reference by #46244: isc_buffer_printf unit test failure added