diff mbox series

[V3,4/8] xen/common: Introduce xrealloc_flex_struct() helper macros

Message ID 1566324587-3442-5-git-send-email-olekstysh@gmail.com (mailing list archive)
State Superseded
Headers show
Series iommu/arm: Add Renesas IPMMU-VMSA support + Linux's iommu_fwspec | expand

Commit Message

Oleksandr Tyshchenko Aug. 20, 2019, 6:09 p.m. UTC 
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

This patch introduces type-safe helper macros to re-allocate space
for a structure with a flexible array of typed objects.

For example, if we need to re-size an array with a single element:

   struct arrlen
   {
      size_t len;
      int data[1];
   };

We can use the proposed macros in the following way:

   new_ptr = realloc_flex_struct(old_ptr, struct arrlen, data, num_elem);

Subsequent patch will use this macros.

Also, while here, introduce xmalloc_flex_struct() to allocate space
for a structure with a flexible array of typed objects.

Suggested-by: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien.grall@arm.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wl@xen.org>
---
 xen/include/xen/xmalloc.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Jan Beulich Aug. 27, 2019, 1:28 p.m. UTC | #1 
On 20.08.2019 20:09, Oleksandr Tyshchenko wrote:
> --- a/xen/include/xen/xmalloc.h
> +++ b/xen/include/xen/xmalloc.h
> @@ -35,6 +35,18 @@
>   #define xzalloc_array(_type, _num) \
>       ((_type *)_xzalloc_array(sizeof(_type), __alignof__(_type), _num))
>   
> +/* Re-allocate space for a structure with a flexible array of typed objects. */
> +#define xrealloc_flex_struct(_ptr, _type, _field, _len) ({                   \

May I ask that you don't extend the bad use of leading underscores here?

> +    /* type checking: make sure that incoming pointer is of correct type */  \

Comment style (should start with upper case char)

> +    (void)((typeof(_ptr)) 0 == (_type *) 0);                                 \

Stray blanks before 0. And why not simply "(void)((ptr) == (type *)0)"?
(You'd need to avoid the double evaluation of ptr, yes.)

> +    ((_type *)_xrealloc(_ptr, offsetof(_type, _field[_len]),                 \
> +                        __alignof__(_type)));                                \

Unnecessary pair of outermost parentheses.

> +})
> +
> +/* Allocate space for a structure with a flexible array of typed objects. */
> +#define xmalloc_flex_struct(_type, _field, _len) \
> +    ((_type *)_xmalloc(offsetof(_type, _field[_len]), __alignof__(_type)))

I think this wants to go ahead of its re-alloc counterpart.

In both cases I'd also like to suggest using "nr" instead of "len",
as something called "length" can be mistaken to represent the
overall length of the resulting object, rather than the number of
array elements.

Jan
Oleksandr Tyshchenko Aug. 28, 2019, 6:23 p.m. UTC | #2 
On 27.08.19 16:28, Jan Beulich wrote:

Hi Jan

> On 20.08.2019 20:09, Oleksandr Tyshchenko wrote:
>> --- a/xen/include/xen/xmalloc.h
>> +++ b/xen/include/xen/xmalloc.h
>> @@ -35,6 +35,18 @@
>>   #define xzalloc_array(_type, _num) \
>>       ((_type *)_xzalloc_array(sizeof(_type), __alignof__(_type), _num))
>>   +/* Re-allocate space for a structure with a flexible array of 
>> typed objects. */
>> +#define xrealloc_flex_struct(_ptr, _type, _field, _len) 
>> ({                   \
>
> May I ask that you don't extend the bad use of leading underscores here?

Yes, sure.


>
>> +    /* type checking: make sure that incoming pointer is of correct 
>> type */  \
>
> Comment style (should start with upper case char)

ok


>
>> +    (void)((typeof(_ptr)) 0 == (_type *) 
>> 0);                                 \
>
> Stray blanks before 0. And why not simply "(void)((ptr) == (type *)0)"?
> (You'd need to avoid the double evaluation of ptr, yes.)

ok


>
>> +    ((_type *)_xrealloc(_ptr, offsetof(_type, 
>> _field[_len]),                 \
>> + __alignof__(_type)));                                \
>
> Unnecessary pair of outermost parentheses.

ok


>
>> +})
>> +
>> +/* Allocate space for a structure with a flexible array of typed 
>> objects. */
>> +#define xmalloc_flex_struct(_type, _field, _len) \
>> +    ((_type *)_xmalloc(offsetof(_type, _field[_len]), 
>> __alignof__(_type)))
>
> I think this wants to go ahead of its re-alloc counterpart.

ok


>
> In both cases I'd also like to suggest using "nr" instead of "len",
> as something called "length" can be mistaken to represent the
> overall length of the resulting object, rather than the number of
> array elements.

sounds reasonable

Thank you.


diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h
index 831152f..3eec10b 100644
--- a/xen/include/xen/xmalloc.h
+++ b/xen/include/xen/xmalloc.h
@@ -35,6 +35,18 @@
  #define xzalloc_array(_type, _num) \
      ((_type *)_xzalloc_array(sizeof(_type), __alignof__(_type), _num))

+/* Allocate space for a structure with a flexible array of typed 
objects. */
+#define xmalloc_flex_struct(type, field, nr) \
+    ((type *)_xmalloc(offsetof(type, field[nr]), __alignof__(type)))
+
+/* Re-allocate space for a structure with a flexible array of typed 
objects. */
+#define xrealloc_flex_struct(ptr, type, field, nr) 
({                        \
+    typeof(*(ptr)) *ptr_ = 
(ptr);                                            \
+    /* Type checking: make sure that incoming pointer is of correct 
type */  \
+    (void)((ptr) == (type 
*)0);                                              \
+    (type *)_xrealloc(ptr_, offsetof(type, field[nr]), 
__alignof__(type));   \
+})
+
  /* Allocate untyped storage. */
  #define xmalloc_bytes(_bytes) _xmalloc(_bytes, SMP_CACHE_BYTES)
  #define xzalloc_bytes(_bytes) _xzalloc(_bytes, SMP_CACHE_BYTES)
(END)
Jan Beulich Aug. 29, 2019, 7:21 a.m. UTC | #3 
On 28.08.2019 20:23, Oleksandr wrote:
> --- a/xen/include/xen/xmalloc.h
> +++ b/xen/include/xen/xmalloc.h
> @@ -35,6 +35,18 @@
>   #define xzalloc_array(_type, _num) \
>       ((_type *)_xzalloc_array(sizeof(_type), __alignof__(_type), _num))
> 
> +/* Allocate space for a structure with a flexible array of typed 
> objects. */
> +#define xmalloc_flex_struct(type, field, nr) \
> +    ((type *)_xmalloc(offsetof(type, field[nr]), __alignof__(type)))
> +
> +/* Re-allocate space for a structure with a flexible array of typed 
> objects. */
> +#define xrealloc_flex_struct(ptr, type, field, nr) 
> ({                        \
> +    typeof(*(ptr)) *ptr_ = 
> (ptr);                                            \
> +    /* Type checking: make sure that incoming pointer is of correct 
> type */  \
> +    (void)((ptr) == (type 
> *)0);                                              \
> +    (type *)_xrealloc(ptr_, offsetof(type, field[nr]), 
> __alignof__(type));   \
> +})
> +

What about

#define xrealloc_flex_struct(ptr, field, nr)                         \
    (typeof(ptr))_xrealloc(ptr, offsetof(typeof(*(ptr)), field[nr]), \
                           __alignof__(typeof(*(ptr))))

?

Jan
Oleksandr Tyshchenko Aug. 29, 2019, 7:04 p.m. UTC | #4 
On 29.08.19 10:21, Jan Beulich wrote:

Hi Jan

> On 28.08.2019 20:23, Oleksandr wrote:
>> --- a/xen/include/xen/xmalloc.h
>> +++ b/xen/include/xen/xmalloc.h
>> @@ -35,6 +35,18 @@
>>    #define xzalloc_array(_type, _num) \
>>        ((_type *)_xzalloc_array(sizeof(_type), __alignof__(_type), _num))
>>
>> +/* Allocate space for a structure with a flexible array of typed
>> objects. */
>> +#define xmalloc_flex_struct(type, field, nr) \
>> +    ((type *)_xmalloc(offsetof(type, field[nr]), __alignof__(type)))
>> +
>> +/* Re-allocate space for a structure with a flexible array of typed
>> objects. */
>> +#define xrealloc_flex_struct(ptr, type, field, nr)
>> ({                        \
>> +    typeof(*(ptr)) *ptr_ =
>> (ptr);                                            \
>> +    /* Type checking: make sure that incoming pointer is of correct
>> type */  \
>> +    (void)((ptr) == (type
>> *)0);                                              \
>> +    (type *)_xrealloc(ptr_, offsetof(type, field[nr]),
>> __alignof__(type));   \
>> +})
>> +
> What about
>
> #define xrealloc_flex_struct(ptr, field, nr)                         \
>      (typeof(ptr))_xrealloc(ptr, offsetof(typeof(*(ptr)), field[nr]), \
>                             __alignof__(typeof(*(ptr))))
>
> ?

Even better. And works...

Thank you.
diff mbox series

Patch

diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h
index 831152f..2eb88a8 100644
--- a/xen/include/xen/xmalloc.h
+++ b/xen/include/xen/xmalloc.h
@@ -35,6 +35,18 @@ 
 #define xzalloc_array(_type, _num) \
     ((_type *)_xzalloc_array(sizeof(_type), __alignof__(_type), _num))
 
+/* Re-allocate space for a structure with a flexible array of typed objects. */
+#define xrealloc_flex_struct(_ptr, _type, _field, _len) ({                   \
+    /* type checking: make sure that incoming pointer is of correct type */  \
+    (void)((typeof(_ptr)) 0 == (_type *) 0);                                 \
+    ((_type *)_xrealloc(_ptr, offsetof(_type, _field[_len]),                 \
+                        __alignof__(_type)));                                \
+})
+
+/* Allocate space for a structure with a flexible array of typed objects. */
+#define xmalloc_flex_struct(_type, _field, _len) \
+    ((_type *)_xmalloc(offsetof(_type, _field[_len]), __alignof__(_type)))
+
 /* Allocate untyped storage. */
 #define xmalloc_bytes(_bytes) _xmalloc(_bytes, SMP_CACHE_BYTES)
 #define xzalloc_bytes(_bytes) _xzalloc(_bytes, SMP_CACHE_BYTES)