From patchwork Mon Apr 8 15:29:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 32201 Received: (qmail 97738 invoked by alias); 8 Apr 2019 15:29:53 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 97729 invoked by uid 89); 8 Apr 2019 15:29:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, UNSUBSCRIBE_BODY autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com From: Florian Weimer To: Carlos O'Donell Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] alloc_buffer: Return unqualified pointer type in alloc_buffer_next References: <20190308204633.49DCE80DD6B5@oldenburg2.str.redhat.com> <6ecb2322-31ad-c5f5-da15-067e67263aa1@redhat.com> Date: Mon, 08 Apr 2019 17:29:47 +0200 In-Reply-To: <6ecb2322-31ad-c5f5-da15-067e67263aa1@redhat.com> (Carlos O'Donell's message of "Mon, 8 Apr 2019 10:34:49 -0400") Message-ID: <878swko6qc.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 * Carlos O'Donell: > On 3/8/19 3:46 PM, Florian Weimer wrote: >> alloc_buffer_next is useful for peeking to the remaining part of the >> buffer and update it, with subsequent allocation (once the length >> is known) using alloc_buffer_alloc_bytes. This is not as robust >> as the other interfaces, but it allows using alloc_buffer with >> string-writing interfaces such as snprintf and ns_name_ntop. > > Until now alloc_buffer_next was only used for testing alloc_buffer itself. > > Please add a detailed example in the comments for how this API should be > used. The use case is interesting enough that it needs comments. > > OK if you add detailed comments in the header for the use case intended. Okay. I think this interface should only be used very sparingly, but I've written a longer comment. Of course, the patch is now substantially larger than before, so it's a bit odd that you pre-approved it. 8-/ Thanks, Florian alloc_buffer: Return unqualified pointer type in alloc_buffer_next alloc_buffer_next is useful for peeking to the remaining part of the buffer and update it, with subsequent allocation (once the length is known) using alloc_buffer_alloc_bytes. This is not as robust as the other interfaces, but it allows using alloc_buffer with string-writing interfaces such as snprintf and ns_name_ntop. 2019-04-08 Florian Weimer * include/alloc_buffer.h (alloc_buffer_alloc_bytes): Update comment. (alloc_buffer_next): Change return type to non-const. Update comment. Reviewed-by: Carlos O'Donell diff --git a/include/alloc_buffer.h b/include/alloc_buffer.h index f27cbb65ca..9c469b9e8b 100644 --- a/include/alloc_buffer.h +++ b/include/alloc_buffer.h @@ -183,7 +183,7 @@ alloc_buffer_add_byte (struct alloc_buffer *buf, unsigned char b) NULL is returned if there is not enough room, and the buffer is marked as failed, or if the buffer has already failed. (Zero-length allocations from an empty buffer which has not yet - failed succeed.) */ + failed succeed.) The buffer contents is not modified. */ static inline __attribute__ ((nonnull (1))) void * alloc_buffer_alloc_bytes (struct alloc_buffer *buf, size_t length) { @@ -300,11 +300,32 @@ __alloc_buffer_next (struct alloc_buffer *buf, size_t align) /* Like alloc_buffer_alloc, but do not advance the pointer beyond the object (so a subseqent call to alloc_buffer_next or alloc_buffer_alloc returns the same pointer). Note that the buffer - is still aligned according to the requirements of TYPE. The effect - of this function is similar to allocating a zero-length array from - the buffer. */ + is still aligned according to the requirements of TYPE, potentially + consuming buffer space. The effect of this function is similar to + allocating a zero-length array from the buffer. + + It is possible to use the return pointer to write to the buffer and + consume the written bytes using alloc_buffer_alloc_bytes (which + does not change the buffer contents), but the calling code needs to + perform manual length checks using alloc_buffer_size. For example, + to read as many int32_t values that are available in the input file + and can fit into the remaining buffer space, you can use this: + + int32_t array = alloc_buffer_next (buf, int32_t); + size_t ret = fread (array, sizeof (int32_t), + alloc_buffer_size (buf) / sizeof (int32_t), fp); + if (ferror (fp)) + handle_error (); + alloc_buffer_alloc_array (buf, int32_t, ret); + + The alloc_buffer_alloc_array call makes the actually-used part of + the buffer permanent. The remaining part of the buffer (not filled + with data from the file) can be used for something else. + + This manual length checking can easily introduce errors, so this + coding style is not recommended. */ #define alloc_buffer_next(buf, type) \ - ((const type *) __alloc_buffer_next \ + ((type *) __alloc_buffer_next \ (buf, __alloc_buffer_assert_align (__alignof__ (type)))) /* Internal function. Allocate an array. */