Re: [Xen-devel] [PATCH V3 4/8] xen/common: Introduce xrealloc_flex_struct() helper macros

[Oleksandr <olekstysh@xxxxxxxxx>]

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)

--
Regards,

Oleksandr Tyshchenko


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel