| Message ID | 20200224142219.30690-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/xmalloc Unify type handling in macros | expand |
On Mon, Feb 24, 2020 at 02:22:19PM +0000, Andrew Cooper wrote: > The macros in xmalloc.h are a mix of using their type parameter directly, and > using typeof(). The only ones I could spot in the neighborhood are xrealloc_flex_struct and xmemdup, which don't have a type parameter but rather a pointer parameter, and hence use typeof against the passed pointer. > Switch uniformly to the latter so expressions can be used, > rather than only type names. I'm fine with this, but I don't think they are a mix, macros using a type parameter clearly expect a type, while macros using ptr expect a pointer, and hence use typeof to get the type. Thanks, Roger.
On 24/02/2020 14:43, Roger Pau Monné wrote: > On Mon, Feb 24, 2020 at 02:22:19PM +0000, Andrew Cooper wrote: >> The macros in xmalloc.h are a mix of using their type parameter directly, and >> using typeof(). > The only ones I could spot in the neighborhood are > xrealloc_flex_struct and xmemdup, which don't have a type parameter > but rather a pointer parameter, and hence use typeof against the > passed pointer. > >> Switch uniformly to the latter so expressions can be used, >> rather than only type names. > I'm fine with this, but I don't think they are a mix, macros > using a type parameter clearly expect a type, while macros using ptr > expect a pointer, and hence use typeof to get the type. I'm afraid this isn't helpful. Its not an ack/nack or any suggestion from a change. The purpose of the change is as stated. Allow expressions as well as typenames. ~Andrew
On Mon, Feb 24, 2020 at 03:49:47PM +0000, Andrew Cooper wrote: > On 24/02/2020 14:43, Roger Pau Monné wrote: > > On Mon, Feb 24, 2020 at 02:22:19PM +0000, Andrew Cooper wrote: > >> The macros in xmalloc.h are a mix of using their type parameter directly, and > >> using typeof(). > > The only ones I could spot in the neighborhood are > > xrealloc_flex_struct and xmemdup, which don't have a type parameter > > but rather a pointer parameter, and hence use typeof against the > > passed pointer. > > > >> Switch uniformly to the latter so expressions can be used, > >> rather than only type names. > > I'm fine with this, but I don't think they are a mix, macros > > using a type parameter clearly expect a type, while macros using ptr > > expect a pointer, and hence use typeof to get the type. > > I'm afraid this isn't helpful. Its not an ack/nack or any suggestion > from a change. Oh, sorry I wasn't clear. I'm not opposed to the change, but I would request a reword of the commit message. Ie: "Allow the macros in xmalloc.h to also support getting passed an instance of a type instead of the type itself. Some macros already expected a pointer to an instance getting passed in, because they had to operate on it, but others only support getting passed a type." Or something along the lines. When I read your commit message it made me think that those macros would randomly expect a type or an instance without any reasoning, but AFAICT there's a reason why things are the way they are now. Thanks, Roger.
On 24.02.2020 15:22, Andrew Cooper wrote: > The macros in xmalloc.h are a mix of using their type parameter directly, and > using typeof(). Switch uniformly to the latter so expressions can be used, > rather than only type names. As Roger says, there's a reason for the split. And as a result of this I'm having trouble seeing how this change is going to be useful. How is a use of these going to look like? void test(void) { struct s *ps; ps = xmalloc(*ps); } is giving the appearance of a deref (not even just a use) of an uninitialized variable. Having seen also your reply to Roger, I think without any mention of what use this is going to be the takeaway from this is rather "let's not do this". I'm sorry, this is unlikely to be what you'd like to hear back, Jan
diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h index f515ceee2a..6ef466b13f 100644 --- a/xen/include/xen/xmalloc.h +++ b/xen/include/xen/xmalloc.h @@ -10,8 +10,11 @@ */ /* Allocate space for typed object. */ -#define xmalloc(_type) ((_type *)_xmalloc(sizeof(_type), __alignof__(_type))) -#define xzalloc(_type) ((_type *)_xzalloc(sizeof(_type), __alignof__(_type))) +#define xmalloc(_type) \ + ((typeof(_type) *)_xmalloc(sizeof(_type), __alignof__(_type))) + +#define xzalloc(_type) \ + ((typeof(_type) *)_xzalloc(sizeof(_type), __alignof__(_type))) /* * Allocate space for a typed object and copy an existing instance. @@ -31,16 +34,16 @@ /* Allocate space for array of typed objects. */ #define xmalloc_array(_type, _num) \ - ((_type *)_xmalloc_array(sizeof(_type), __alignof__(_type), _num)) + ((typeof(_type) *)_xmalloc_array(sizeof(_type), __alignof__(_type), _num)) #define xzalloc_array(_type, _num) \ - ((_type *)_xzalloc_array(sizeof(_type), __alignof__(_type), _num)) + ((typeof(_type) *)_xzalloc_array(sizeof(_type), __alignof__(_type), _num)) /* Allocate space for a structure with a flexible array of typed objects. */ #define xzalloc_flex_struct(type, field, nr) \ - ((type *)_xzalloc(offsetof(type, field[nr]), __alignof__(type))) + ((typeof(type) *)_xzalloc(offsetof(type, field[nr]), __alignof__(type))) #define xmalloc_flex_struct(type, field, nr) \ - ((type *)_xmalloc(offsetof(type, field[nr]), __alignof__(type))) + ((typeof(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, field, nr) \
The macros in xmalloc.h are a mix of using their type parameter directly, and using typeof(). Switch uniformly to the latter so expressions can be used, rather than only type names. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wl@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/include/xen/xmalloc.h | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)