diff mbox

[4/9] Sync scratch_buffer with gnulib

Message ID f65983a5-6e0e-c93e-8cd7-4bb5ee7f1172@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Netto Sept. 18, 2017, 11:43 a.m. UTC 
  On 18/09/2017 03:09, Florian Weimer wrote:
> On 09/05/2017 10:25 PM, Adhemerval Zanella wrote:
>> -  char __space[1024]
>> -    __attribute__ ((aligned (__alignof__ (max_align_t))));
>> +  max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
> 
> This change needs a declaration from the GCC folks (probably from Richard Biener) that it does not break aliasing analysis.  The old code uses a GCC extension (writing to a char array changes its dynamic type); it is not valid C.  I don't know if the GCC extension also applies if the storage site is declared with a non-char type.
> 
> Thanks,
> Florian


What about this below instead:

---


---

_Alignas/stdalign.h is supported since GCC 4.7 [1] (so it is safe to use
on glibc without configure support) and gnulib have its own version which
defines __alignas_is_defined depending on underlying compiler support.

[1] https://gcc.gnu.org/wiki/C11Status

Comments

Florian Weimer Sept. 18, 2017, 11:57 a.m. UTC | #1 
On 09/18/2017 01:43 PM, Adhemerval Zanella wrote:
> +#if __alignas_is_defined
> +  _Alignas (max_align_t) char __space[1024];
> +#else
>     max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
> +#endif

This works for me on the glibc side.

By the way, do you have a rebased version of this patch?

Subject: [PATCH 05/18] posix: Rewrite to use struct scratch_buffer 
instead of extend_alloca

Thanks,
Florian
Adhemerval Zanella Netto Sept. 18, 2017, 12:25 p.m. UTC | #2 
On 18/09/2017 08:57, Florian Weimer wrote:
> On 09/18/2017 01:43 PM, Adhemerval Zanella wrote:
>> +#if __alignas_is_defined
>> +  _Alignas (max_align_t) char __space[1024];
>> +#else
>>     max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
>> +#endif
> 
> This works for me on the glibc side.

Right, I will prepare a patch then.

> 
> By the way, do you have a rebased version of this patch?
> 
> Subject: [PATCH 05/18] posix: Rewrite to use struct scratch_buffer instead of extend_alloca
> 
> Thanks,
> Florian

No, but I can send along with some more glob fixes I plan to.
diff mbox

Patch

diff --git a/include/scratch_buffer.h b/include/scratch_buffer.h
index bb04662..f77e7da 100644
--- a/include/scratch_buffer.h
+++ b/include/scratch_buffer.h
@@ -60,13 +60,18 @@ 
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdlib.h>
+#include <stdalign.h>
 
 /* Scratch buffer.  Must be initialized with scratch_buffer_init
    before its use.  */
 struct scratch_buffer {
   void *data;    /* Pointer to the beginning of the scratch area.  */
   size_t length; /* Allocated space at the data pointer, in bytes.  */
+#if __alignas_is_defined
+  _Alignas (max_align_t) char __space[1024];
+#else
   max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
+#endif
 };
 
 /* Initializes *BUFFER so that BUFFER->data points to BUFFER->__space