Re: [PATCH 2/2] Use C11 _Alignas on scratch_buffer internal buffer

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

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.

---

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);
-- 
2.7.4