[2/2] Use C11 _Alignas on scratch_buffer internal buffer
Commit Message
On 18/09/2017 14:24, Paul Eggert wrote:
> Florian Weimer wrote:
>>> Sorry, didn't know that. We could use 1056-byte buffers instead; would that be the same on all glibc platforms? I'm not sure how important it is that the byte count be the same everywhere.
>>
>> I've chased ghosts due to such buffer size differences. I'd like to reduce it to the 32/64 difference if possible.
>
> Then Andreas's suggestion is a good way to go. A member like this in struct scratch_buffer:
>
> union { max_align_t __a; char __c[1024]; } __space;
>
> and then use __space.__c everywhere the code currently uses __space. This will make the buffer size 1024 on all platforms where sizeof (max_align_t) <= 1024, which should be a safe assumption for all glibc platforms now.
Right, the union does seems a better option. What about:
---
* include/scratch_buffer.h (scratch_bufferi, scratch_buffer_init,
scratch_buffer_free): Use an union instead of a max_align_t array
for __space definition.
* malloc/scratch_buffer_grow_preserve.c
(__libc_scratch_buffer_grow_preserve): Likewise.
* malloc/tst-scratch_buffer.c (array_size_must_fail): Likewise.
---
Comments
On Sep 19 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> diff --git a/include/scratch_buffer.h b/include/scratch_buffer.h
> index bb04662..0f49dd9 100644
> --- a/include/scratch_buffer.h
> +++ b/include/scratch_buffer.h
> @@ -66,7 +66,7 @@
> struct scratch_buffer {
> void *data; /* Pointer to the beginning of the scratch area. */
> size_t length; /* Allocated space at the data pointer, in bytes. */
> - max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
> + union { max_align_t __a; char __c[1024]; } __space;
If you make it an anonymous union you don't even need to change the rest
of the code.
Andreas.
@@ -66,7 +66,7 @@
struct scratch_buffer {
void *data; /* Pointer to the beginning of the scratch area. */
size_t length; /* Allocated space at the data pointer, in bytes. */
- max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
+ union { max_align_t __a; char __c[1024]; } __space;
};
/* Initializes *BUFFER so that BUFFER->data points to BUFFER->__space
@@ -74,7 +74,7 @@ struct scratch_buffer {
static inline void
scratch_buffer_init (struct scratch_buffer *buffer)
{
- buffer->data = buffer->__space;
+ buffer->data = buffer->__space.__c;
buffer->length = sizeof (buffer->__space);
}
@@ -82,7 +82,7 @@ scratch_buffer_init (struct scratch_buffer *buffer)
static inline void
scratch_buffer_free (struct scratch_buffer *buffer)
{
- if (buffer->data != buffer->__space)
+ if (buffer->data != buffer->__space.__c)
free (buffer->data);
}
@@ -30,14 +30,14 @@ __libc_scratch_buffer_grow_preserve (struct scratch_buffer *buffer)
size_t new_length = 2 * buffer->length;
void *new_ptr;
- if (buffer->data == buffer->__space)
+ if (buffer->data == buffer->__space.__c)
{
/* 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);
+ memcpy (new_ptr, buffer->__space.__c, buffer->length);
}
else
{
@@ -59,7 +59,7 @@ array_size_must_fail (size_t a, size_t b)
pass, a, b);
return false;
}
- if (buf.data != buf.__space)
+ if (buf.data != buf.__space.__c)
{
printf ("scratch_buffer_set_array_size did not free: %d %zu %zu\n",
pass, a, b);