From patchwork Fri Jun 2 12:00:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 20697 Received: (qmail 91854 invoked by alias); 2 Jun 2017 12:01:00 -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 91415 invoked by uid 89); 2 Jun 2017 12:00:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BBB047E9F6 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=fweimer@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com BBB047E9F6 Date: Fri, 02 Jun 2017 14:00:15 +0200 To: libc-alpha@sourceware.org Subject: [PATCH] scratch_buffer: Remove functionality superseded by dynamic arrays User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20170602120015.0B6A5401268B1@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) 2017-06-02 Florian Weimer * include/scratch_buffer.h: Update comments. (__libc_scratch_buffer_grow_preserve): Remove declaration. (scratch_buffer_grow_preserve): Remove definition. (__libc_scratch_buffer_set_array_size): Remove declaration. (scratch_buffer_set_array_size): Remove definition. * malloc/Makefile (routines): Remove scratch_buffer_grow_preserve, scratch_buffer_set_array_size. * malloc/Versions (GLIBC_PRIVATE): Remove __libc_scratch_buffer_grow_preserve, __libc_scratch_buffer_set_array_size. * malloc/dynarray.h: Update comment. * malloc/scratch_buffer_grow_preserve.c: Remove file. * malloc/scratch_buffer_set_array_size.c: Likewise. * malloc/tst-scratch_buffer.c: Remove tests for dropped functionality. diff --git a/include/scratch_buffer.h b/include/scratch_buffer.h index dd17a4a..8306c9e 100644 --- a/include/scratch_buffer.h +++ b/include/scratch_buffer.h @@ -33,12 +33,11 @@ scratch_buffer_free (&tmpbuf); return 0; - The allocation functions (scratch_buffer_grow, - scratch_buffer_grow_preserve, scratch_buffer_set_array_size) make - sure that the heap allocation, if any, is freed, so that the code - above does not have a memory leak. The buffer still remains in a - state that can be deallocated using scratch_buffer_free, so a loop - like this is valid as well: + The allocation function (scratch_buffer_grow) ensures that the heap + allocation, if any, is freed, so that the code above does not have + a memory leak. The buffer still remains in a state that can be + deallocated using scratch_buffer_free, so a loop like this is valid + as well: struct scratch_buffer tmpbuf; scratch_buffer_init (&tmpbuf); @@ -49,12 +48,11 @@ scratch_buffer_free (&tmpbuf); - scratch_buffer_grow and scratch_buffer_grow_preserve are guaranteed - to grow the buffer by at least 512 bytes. This means that when - using the scratch buffer as a backing store for a non-character - array whose element size, in bytes, is 512 or smaller, the scratch - buffer only has to grow once to make room for at least one more - element. + scratch_buffer_grow is guaranteed to grow the buffer by at least + 512 bytes. This means that when using the scratch buffer as a + backing store for a non-character array whose element size, in + bytes, is 512 or smaller, the scratch buffer only has to grow once + to make room for at least one more element. */ #include @@ -101,36 +99,4 @@ scratch_buffer_grow (struct scratch_buffer *buffer) { return __glibc_likely (__libc_scratch_buffer_grow (buffer)); } - -/* Like __libc_scratch_buffer_grow, but preserve the old buffer - contents on success, as a prefix of the new buffer. */ -bool __libc_scratch_buffer_grow_preserve (struct scratch_buffer *buffer); -libc_hidden_proto (__libc_scratch_buffer_grow_preserve) - -/* Alias for __libc_scratch_buffer_grow_preserve. */ -static __always_inline bool -scratch_buffer_grow_preserve (struct scratch_buffer *buffer) -{ - return __glibc_likely (__libc_scratch_buffer_grow_preserve (buffer)); -} - -/* Grow *BUFFER so that it can store at least NELEM elements of SIZE - bytes. The buffer contents are NOT preserved. Both NELEM and SIZE - can be zero. Return true on success, false on allocation failure - (in which case the old buffer is freed, but *BUFFER remains in a - free-able state, and errno is set). It is unspecified whether this - function can reduce the array size. */ -bool __libc_scratch_buffer_set_array_size (struct scratch_buffer *buffer, - size_t nelem, size_t size); -libc_hidden_proto (__libc_scratch_buffer_set_array_size) - -/* Alias for __libc_scratch_set_array_size. */ -static __always_inline bool -scratch_buffer_set_array_size (struct scratch_buffer *buffer, - size_t nelem, size_t size) -{ - return __glibc_likely (__libc_scratch_buffer_set_array_size - (buffer, nelem, size)); -} - #endif /* _SCRATCH_BUFFER_H */ diff --git a/malloc/Makefile b/malloc/Makefile index af025cb..7c3538c 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -56,8 +56,7 @@ tests += $(tests-static) test-srcs = tst-mtrace tst-dynarray tst-dynarray-fail routines = malloc morecore mcheck mtrace obstack reallocarray \ - scratch_buffer_grow scratch_buffer_grow_preserve \ - scratch_buffer_set_array_size \ + scratch_buffer_grow \ dynarray_at_failure \ dynarray_emplace_enlarge \ dynarray_finalize \ diff --git a/malloc/Versions b/malloc/Versions index 5b54306..df8602e 100644 --- a/malloc/Versions +++ b/malloc/Versions @@ -73,9 +73,6 @@ libc { # struct scratch_buffer support __libc_scratch_buffer_grow; - __libc_scratch_buffer_grow_preserve; - __libc_scratch_buffer_set_array_size; - # Internal name for reallocarray __libc_reallocarray; diff --git a/malloc/dynarray.h b/malloc/dynarray.h index c73e08b..db3ce24 100644 --- a/malloc/dynarray.h +++ b/malloc/dynarray.h @@ -79,10 +79,7 @@ - They have an element type, and are not just an untyped buffer of bytes. - - When growing, previously stored elements are preserved. (It is - expected that scratch_buffer_grow_preserve and - scratch_buffer_set_array_size eventually go away because all - current users are moved to dynamic arrays.) + - When growing, previously stored elements are preserved. - Scratch buffers have a more aggressive growth policy because growing them typically means a retry of an operation (across an diff --git a/malloc/scratch_buffer_grow_preserve.c b/malloc/scratch_buffer_grow_preserve.c deleted file mode 100644 index 18543ef..0000000 --- a/malloc/scratch_buffer_grow_preserve.c +++ /dev/null @@ -1,63 +0,0 @@ -/* Variable-sized buffer with on-stack default allocation. - Copyright (C) 2015-2017 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#include -#include -#include - -bool -__libc_scratch_buffer_grow_preserve (struct scratch_buffer *buffer) -{ - size_t new_length = 2 * buffer->length; - void *new_ptr; - - if (buffer->data == buffer->__space) - { - /* Move buffer to the heap. No overflow is possible because - buffer->length describes a small buffer on the stack. */ - new_ptr = malloc (new_length); - if (new_ptr == NULL) - return false; - memcpy (new_ptr, buffer->__space, buffer->length); - } - else - { - /* Buffer was already on the heap. Check for overflow. */ - if (__glibc_likely (new_length >= buffer->length)) - new_ptr = realloc (buffer->data, new_length); - else - { - __set_errno (ENOMEM); - new_ptr = NULL; - } - - if (__glibc_unlikely (new_ptr == NULL)) - { - /* Deallocate, but buffer must remain valid to free. */ - free (buffer->data); - scratch_buffer_init (buffer); - return false; - } - } - - /* Install new heap-based buffer. */ - buffer->data = new_ptr; - buffer->length = new_length; - return true; -} -libc_hidden_def (__libc_scratch_buffer_grow_preserve); diff --git a/malloc/scratch_buffer_set_array_size.c b/malloc/scratch_buffer_set_array_size.c deleted file mode 100644 index 8ab6d9d..0000000 --- a/malloc/scratch_buffer_set_array_size.c +++ /dev/null @@ -1,60 +0,0 @@ -/* Variable-sized buffer with on-stack default allocation. - Copyright (C) 2015-2017 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#include -#include -#include - -bool -__libc_scratch_buffer_set_array_size (struct scratch_buffer *buffer, - size_t nelem, size_t size) -{ - size_t new_length = nelem * size; - - /* Avoid overflow check if both values are small. */ - if ((nelem | size) >> (sizeof (size_t) * CHAR_BIT / 2) != 0 - && nelem != 0 && size != new_length / nelem) - { - /* Overflow. Discard the old buffer, but it must remain valid - to free. */ - scratch_buffer_free (buffer); - scratch_buffer_init (buffer); - __set_errno (ENOMEM); - return false; - } - - if (new_length <= buffer->length) - return true; - - /* Discard old buffer. */ - scratch_buffer_free (buffer); - - char *new_ptr = malloc (new_length); - if (new_ptr == NULL) - { - /* Buffer must remain valid to free. */ - scratch_buffer_init (buffer); - return false; - } - - /* Install new heap-based buffer. */ - buffer->data = new_ptr; - buffer->length = new_length; - return true; -} -libc_hidden_def (__libc_scratch_buffer_set_array_size); diff --git a/malloc/tst-scratch_buffer.c b/malloc/tst-scratch_buffer.c index 5c9f344..921624f 100644 --- a/malloc/tst-scratch_buffer.c +++ b/malloc/tst-scratch_buffer.c @@ -21,54 +21,6 @@ #include #include -static bool -unchanged_array_size (struct scratch_buffer *buf, size_t a, size_t b) -{ - size_t old_length = buf->length; - if (!scratch_buffer_set_array_size (buf, a, b)) - { - printf ("scratch_buffer_set_array_size failed: %zu %zu\n", - a, b); - return false; - } - if (old_length != buf->length) - { - printf ("scratch_buffer_set_array_size did not preserve size: %zu %zu\n", - a, b); - return false; - } - return true; -} - -static bool -array_size_must_fail (size_t a, size_t b) -{ - for (int pass = 0; pass < 2; ++pass) - { - struct scratch_buffer buf; - scratch_buffer_init (&buf); - if (pass > 0) - if (!scratch_buffer_grow (&buf)) - { - printf ("scratch_buffer_grow in array_size_must_fail failed\n"); - return false; - } - if (scratch_buffer_set_array_size (&buf, a, b)) - { - printf ("scratch_buffer_set_array_size passed: %d %zu %zu\n", - pass, a, b); - return false; - } - if (buf.data != buf.__space) - { - printf ("scratch_buffer_set_array_size did not free: %d %zu %zu\n", - pass, a, b); - return false; - } - } - return true; -} - static int do_test (void) { @@ -92,62 +44,6 @@ do_test (void) memset (buf.data, ' ', buf.length); scratch_buffer_free (&buf); } - { - struct scratch_buffer buf; - scratch_buffer_init (&buf); - memset (buf.data, '@', buf.length); - strcpy (buf.data, "prefix"); - size_t old_length = buf.length; - scratch_buffer_grow_preserve (&buf); - if (buf.length <= old_length) - { - printf ("scratch_buffer_grow_preserve did not enlarge buffer\n"); - return 1; - } - if (strcmp (buf.data, "prefix") != 0) - { - printf ("scratch_buffer_grow_preserve did not copy buffer\n"); - return 1; - } - for (unsigned i = 7; i < old_length; ++i) - if (((char *)buf.data)[i] != '@') - { - printf ("scratch_buffer_grow_preserve did not copy buffer (%u)\n", - i); - return 1; - } - scratch_buffer_free (&buf); - } - { - struct scratch_buffer buf; - scratch_buffer_init (&buf); - for (int pass = 0; pass < 4; ++pass) - { - if (!(unchanged_array_size (&buf, 0, 0) - && unchanged_array_size (&buf, 1, 0) - && unchanged_array_size (&buf, 0, 1) - && unchanged_array_size (&buf, -1, 0) - && unchanged_array_size (&buf, 0, -1) - && unchanged_array_size (&buf, 1ULL << 16, 0) - && unchanged_array_size (&buf, 0, 1ULL << 16) - && unchanged_array_size (&buf, (size_t) (1ULL << 32), 0) - && unchanged_array_size (&buf, 0, (size_t) (1ULL << 32)))) - return 1; - if (!scratch_buffer_grow (&buf)) - { - printf ("scratch_buffer_grow_failed (pass %d)\n", pass); - } - } - scratch_buffer_free (&buf); - } - { - if (!(array_size_must_fail (-1, 1) - && array_size_must_fail (-1, -1) - && array_size_must_fail (1, -1) - && array_size_must_fail (((size_t)-1) / 4, 4) - && array_size_must_fail (4, ((size_t)-1) / 4))) - return 1; - } return 0; }