diff mbox

[2/2] Use C11 _Alignas on scratch_buffer internal buffer

Message ID 30c017fe-fafe-d787-7d76-865223cef599@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Netto Sept. 19, 2017, 1:08 p.m. UTC 
  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

Andreas Schwab Sept. 19, 2017, 1:29 p.m. UTC | #1 
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.
diff mbox

Patch

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;
 };
 
 /* 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);
 }
 
diff --git a/malloc/scratch_buffer_grow_preserve.c b/malloc/scratch_buffer_grow_preserve.c
index 9268615..59a922b 100644
--- a/malloc/scratch_buffer_grow_preserve.c
+++ b/malloc/scratch_buffer_grow_preserve.c
@@ -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
     {
diff --git a/malloc/tst-scratch_buffer.c b/malloc/tst-scratch_buffer.c
index 5c9f344..86447b6 100644
--- a/malloc/tst-scratch_buffer.c
+++ b/malloc/tst-scratch_buffer.c
@@ -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);